Message ID | 20220715150219.16875-1-sreekanth.reddy@broadcom.com |
---|---|
Headers | show |
Series | mpi3mr: Fix compilation errors on i386 arch | expand |
Hi Guenter, Please check the changes below. I hope this change will work with 32-bit pointers as well. If it looks good then I will post this change as a patch. diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c index 0901bc932d5c..0bba19c0f984 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_os.c +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct mpi3mr_ioc *mrioc, ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); return; } - *(__le64 *)fwevt->event_data = (__le64)tg; + memcpy(fwevt->event_data, (char *)&tg, sizeof(void *)); fwevt->mrioc = mrioc; fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; fwevt->send_ack = 0; @@ -1660,8 +1660,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, { struct mpi3mr_throttle_group_info *tg; - tg = (struct mpi3mr_throttle_group_info *) - (*(__le64 *)fwevt->event_data); + memcpy((char *)&tg, fwevt->event_data, sizeof(void *)); dprint_event_bh(mrioc, "qd reduction event processed for tg_id(%d) reduction_needed(%d)\n", tg->id, tg->need_qd_reduction); Thanks, Sreekanth On Fri, Jul 15, 2022 at 10:19 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 7/15/22 08:02, Sreekanth Reddy wrote: > > Fix below compilation errors observed on i386 ARCH, > > > > cast from pointer to integer of different size > > [-Werror=pointer-to-int-cast] > > > > Fixes: c196bc4dce ("scsi: mpi3mr: Reduce VD queue depth on detecting throttling") > > Reported-by: Guenter Roeck <linux@roeck-us.net> > > Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com> > > --- > > drivers/scsi/mpi3mr/mpi3mr_os.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > > index 0901bc932d5c..d8013576d863 100644 > > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > > @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct mpi3mr_ioc *mrioc, > > ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); > > return; > > } > > - *(__le64 *)fwevt->event_data = (__le64)tg; > > + memcpy(fwevt->event_data, (char *)&tg, sizeof(u64)); > > fwevt->mrioc = mrioc; > > fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; > > fwevt->send_ack = 0; > > @@ -1660,8 +1660,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, > > { > > struct mpi3mr_throttle_group_info *tg; > > > > - tg = (struct mpi3mr_throttle_group_info *) > > - (*(__le64 *)fwevt->event_data); > > + memcpy((char *)&tg, fwevt->event_data, sizeof(u64)); > > How is this expected to work on a system with 32-bit pointers ? > > Guenter > > > dprint_event_bh(mrioc, > > "qd reduction event processed for tg_id(%d) reduction_needed(%d)\n", > > tg->id, tg->need_qd_reduction); >
On 7/16/22 06:35, Sreekanth Reddy wrote: > Please check the changes below. I hope this change will work with > 32-bit pointers as well. If it looks good then I will post this > change as a patch. > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > index 0901bc932d5c..0bba19c0f984 100644 > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct > mpi3mr_ioc *mrioc, > ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); > return; > } > - *(__le64 *)fwevt->event_data = (__le64)tg; > + memcpy(fwevt->event_data, (char *)&tg, sizeof(void *)); > fwevt->mrioc = mrioc; > fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; > fwevt->send_ack = 0; > @@ -1660,8 +1660,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, > { > struct mpi3mr_throttle_group_info *tg; > > - tg = (struct mpi3mr_throttle_group_info *) > - (*(__le64 *)fwevt->event_data); > + memcpy((char *)&tg, fwevt->event_data, sizeof(void *)); > dprint_event_bh(mrioc, > "qd reduction event processed for tg_id(%d) > reduction_needed(%d)\n", > tg->id, tg->need_qd_reduction); How about reverting c196bc4dce ("scsi: mpi3mr: Reduce VD queue depth on detecting throttling") to remove the time pressure for coming up with a fix for that commit? Thanks, Bart.
On 7/16/22 06:35, Sreekanth Reddy wrote: > Hi Guenter, > > Please check the changes below. I hope this change will work with > 32-bit pointers as well. If it looks good then I will post this > change as a patch. > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > index 0901bc932d5c..0bba19c0f984 100644 > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct > mpi3mr_ioc *mrioc, > ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); > return; > } > - *(__le64 *)fwevt->event_data = (__le64)tg; > + memcpy(fwevt->event_data, (char *)&tg, sizeof(void *)); > fwevt->mrioc = mrioc; > fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; > fwevt->send_ack = 0; > @@ -1660,8 +1660,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, > { > struct mpi3mr_throttle_group_info *tg; > > - tg = (struct mpi3mr_throttle_group_info *) > - (*(__le64 *)fwevt->event_data); > + memcpy((char *)&tg, fwevt->event_data, sizeof(void *)); > dprint_event_bh(mrioc, > "qd reduction event processed for tg_id(%d) > reduction_needed(%d)\n", > tg->id, tg->need_qd_reduction); > If I understand correctly, you want to pass the pointer to tg along. If so, the following seems cleaner and less confusing to me. diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c index 6a47f8c77256..f581c07c2665 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_os.c +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct mpi3mr_ioc *mrioc, ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); return; } - *(__le64 *)fwevt->event_data = (__le64)tg; + *(struct mpi3mr_throttle_group_info **)fwevt->event_data = tg; fwevt->mrioc = mrioc; fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; fwevt->send_ack = 0; @@ -1652,8 +1652,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, { struct mpi3mr_throttle_group_info *tg; - tg = (struct mpi3mr_throttle_group_info *) - (*(__le64 *)fwevt->event_data); + tg = *(struct mpi3mr_throttle_group_info **)fwevt->event_data; dprint_event_bh(mrioc, "qd reduction event processed for tg_id(%d) reduction_needed(%d)\n", tg->id, tg->need_qd_reduction); or simply diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c index 6a47f8c77256..cc93b41dd428 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_os.c +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct mpi3mr_ioc *mrioc, ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); return; } - *(__le64 *)fwevt->event_data = (__le64)tg; + *(void **)fwevt->event_data = tg; fwevt->mrioc = mrioc; fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; fwevt->send_ack = 0; @@ -1652,8 +1652,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, { struct mpi3mr_throttle_group_info *tg; - tg = (struct mpi3mr_throttle_group_info *) - (*(__le64 *)fwevt->event_data); + tg = *(void **)fwevt->event_data; dprint_event_bh(mrioc, "qd reduction event processed for tg_id(%d) reduction_needed(%d)\n", tg->id, tg->need_qd_reduction); Thanks, Guenter > Thanks, > Sreekanth > > On Fri, Jul 15, 2022 at 10:19 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 7/15/22 08:02, Sreekanth Reddy wrote: >>> Fix below compilation errors observed on i386 ARCH, >>> >>> cast from pointer to integer of different size >>> [-Werror=pointer-to-int-cast] >>> >>> Fixes: c196bc4dce ("scsi: mpi3mr: Reduce VD queue depth on detecting throttling") >>> Reported-by: Guenter Roeck <linux@roeck-us.net> >>> Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com> >>> --- >>> drivers/scsi/mpi3mr/mpi3mr_os.c | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c >>> index 0901bc932d5c..d8013576d863 100644 >>> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c >>> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c >>> @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct mpi3mr_ioc *mrioc, >>> ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); >>> return; >>> } >>> - *(__le64 *)fwevt->event_data = (__le64)tg; >>> + memcpy(fwevt->event_data, (char *)&tg, sizeof(u64)); >>> fwevt->mrioc = mrioc; >>> fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; >>> fwevt->send_ack = 0; >>> @@ -1660,8 +1660,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, >>> { >>> struct mpi3mr_throttle_group_info *tg; >>> >>> - tg = (struct mpi3mr_throttle_group_info *) >>> - (*(__le64 *)fwevt->event_data); >>> + memcpy((char *)&tg, fwevt->event_data, sizeof(u64)); >> >> How is this expected to work on a system with 32-bit pointers ? >> >> Guenter >> >>> dprint_event_bh(mrioc, >>> "qd reduction event processed for tg_id(%d) reduction_needed(%d)\n", >>> tg->id, tg->need_qd_reduction); >>
On 7/16/22 06:45, Bart Van Assche wrote: > On 7/16/22 06:35, Sreekanth Reddy wrote: >> Please check the changes below. I hope this change will work with >> 32-bit pointers as well. If it looks good then I will post this >> change as a patch. >> >> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c >> index 0901bc932d5c..0bba19c0f984 100644 >> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c >> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c >> @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct >> mpi3mr_ioc *mrioc, >> ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); >> return; >> } >> - *(__le64 *)fwevt->event_data = (__le64)tg; >> + memcpy(fwevt->event_data, (char *)&tg, sizeof(void *)); >> fwevt->mrioc = mrioc; >> fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; >> fwevt->send_ack = 0; >> @@ -1660,8 +1660,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, >> { >> struct mpi3mr_throttle_group_info *tg; >> >> - tg = (struct mpi3mr_throttle_group_info *) >> - (*(__le64 *)fwevt->event_data); >> + memcpy((char *)&tg, fwevt->event_data, sizeof(void *)); >> dprint_event_bh(mrioc, >> "qd reduction event processed for tg_id(%d) >> reduction_needed(%d)\n", >> tg->id, tg->need_qd_reduction); > > How about reverting c196bc4dce ("scsi: mpi3mr: Reduce VD queue depth on detecting throttling") to remove the time pressure for coming up with a fix for that commit? > Fortunately this is only seen in linux-next, so there would still be a week or two to do that. Really, I am happy that this is looked at - we still have build regressions from the last merge window in v5.19-rc6, and it sometimes takes Linus to intervene to get things fixed there. Thanks, Guenter
On Sat, Jul 16, 2022 at 7:34 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 7/16/22 06:35, Sreekanth Reddy wrote: > > Hi Guenter, > > > > Please check the changes below. I hope this change will work with > > 32-bit pointers as well. If it looks good then I will post this > > change as a patch. > > > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > > index 0901bc932d5c..0bba19c0f984 100644 > > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > > @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct > > mpi3mr_ioc *mrioc, > > ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); > > return; > > } > > - *(__le64 *)fwevt->event_data = (__le64)tg; > > + memcpy(fwevt->event_data, (char *)&tg, sizeof(void *)); > fwevt->mrioc = mrioc; > > fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; > > fwevt->send_ack = 0; > > @@ -1660,8 +1660,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, > > { > > struct mpi3mr_throttle_group_info *tg; > > > > - tg = (struct mpi3mr_throttle_group_info *) > > - (*(__le64 *)fwevt->event_data); > > + memcpy((char *)&tg, fwevt->event_data, sizeof(void *)); > > dprint_event_bh(mrioc, > > "qd reduction event processed for tg_id(%d) > > reduction_needed(%d)\n", > > tg->id, tg->need_qd_reduction); > > > > If I understand correctly, you want to pass the pointer to tg along. If so, > the following seems cleaner and less confusing to me. Yes, it is correct. I have posted a new patch with your suggested changes below. https://patchwork.kernel.org/project/linux-scsi/patch/20220718095351.15868-2-sreekanth.reddy@broadcom.com/ Thanks, Sreekanth > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > index 6a47f8c77256..f581c07c2665 100644 > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct mpi3mr_ioc *mrioc, > ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); > return; > } > - *(__le64 *)fwevt->event_data = (__le64)tg; > + *(struct mpi3mr_throttle_group_info **)fwevt->event_data = tg; > fwevt->mrioc = mrioc; > fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; > fwevt->send_ack = 0; > @@ -1652,8 +1652,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, > { > struct mpi3mr_throttle_group_info *tg; > > - tg = (struct mpi3mr_throttle_group_info *) > - (*(__le64 *)fwevt->event_data); > + tg = *(struct mpi3mr_throttle_group_info **)fwevt->event_data; > dprint_event_bh(mrioc, > "qd reduction event processed for tg_id(%d) reduction_needed(%d)\n", > tg->id, tg->need_qd_reduction); > > or simply > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > index 6a47f8c77256..cc93b41dd428 100644 > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct mpi3mr_ioc *mrioc, > ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); > return; > } > - *(__le64 *)fwevt->event_data = (__le64)tg; > + *(void **)fwevt->event_data = tg; > fwevt->mrioc = mrioc; > fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; > fwevt->send_ack = 0; > @@ -1652,8 +1652,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, > { > struct mpi3mr_throttle_group_info *tg; > > - tg = (struct mpi3mr_throttle_group_info *) > - (*(__le64 *)fwevt->event_data); > + tg = *(void **)fwevt->event_data; > dprint_event_bh(mrioc, > "qd reduction event processed for tg_id(%d) reduction_needed(%d)\n", > tg->id, tg->need_qd_reduction); > > Thanks, > Guenter > > > Thanks, > > Sreekanth > > > > On Fri, Jul 15, 2022 at 10:19 PM Guenter Roeck <linux@roeck-us.net> wrote: > >> > >> On 7/15/22 08:02, Sreekanth Reddy wrote: > >>> Fix below compilation errors observed on i386 ARCH, > >>> > >>> cast from pointer to integer of different size > >>> [-Werror=pointer-to-int-cast] > >>> > >>> Fixes: c196bc4dce ("scsi: mpi3mr: Reduce VD queue depth on detecting throttling") > >>> Reported-by: Guenter Roeck <linux@roeck-us.net> > >>> Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com> > >>> --- > >>> drivers/scsi/mpi3mr/mpi3mr_os.c | 5 ++--- > >>> 1 file changed, 2 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > >>> index 0901bc932d5c..d8013576d863 100644 > >>> --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > >>> +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > >>> @@ -386,7 +386,7 @@ static void mpi3mr_queue_qd_reduction_event(struct mpi3mr_ioc *mrioc, > >>> ioc_warn(mrioc, "failed to queue TG QD reduction event\n"); > >>> return; > >>> } > >>> - *(__le64 *)fwevt->event_data = (__le64)tg; > >>> + memcpy(fwevt->event_data, (char *)&tg, sizeof(u64)); > >>> fwevt->mrioc = mrioc; > >>> fwevt->event_id = MPI3MR_DRIVER_EVENT_TG_QD_REDUCTION; > >>> fwevt->send_ack = 0; > >>> @@ -1660,8 +1660,7 @@ static void mpi3mr_fwevt_bh(struct mpi3mr_ioc *mrioc, > >>> { > >>> struct mpi3mr_throttle_group_info *tg; > >>> > >>> - tg = (struct mpi3mr_throttle_group_info *) > >>> - (*(__le64 *)fwevt->event_data); > >>> + memcpy((char *)&tg, fwevt->event_data, sizeof(u64)); > >> > >> How is this expected to work on a system with 32-bit pointers ? > >> > >> Guenter > >> > >>> dprint_event_bh(mrioc, > >>> "qd reduction event processed for tg_id(%d) reduction_needed(%d)\n", > >>> tg->id, tg->need_qd_reduction); > >> >