| Message ID | 20200817085409.25268-1-allen.cryptic@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | scsi: convert tasklets to use new tasklet_setup() | expand |
On Mon, 2020-08-17 at 14:24 +0530, Allen Pais wrote: > From: Allen Pais <allen.lkml@gmail.com> > > Commit 12cc923f1ccc ("tasklet: Introduce new initialization API")' > introduced a new tasklet initialization API. This series converts > all the scsi drivers to use the new tasklet_setup() API I've got to say I agree with Jens, this was a silly obfuscation: +#define from_tasklet(var, callback_tasklet, tasklet_fieldname) \ + container_of(callback_tasklet, typeof(*var), tasklet_fieldname) Just use container_of directly since we all understand what it does. James
On Mon, Aug 17, 2020 at 07:41:58AM -0700, James Bottomley wrote: > On Mon, 2020-08-17 at 14:24 +0530, Allen Pais wrote: > > From: Allen Pais <allen.lkml@gmail.com> > > > > Commit 12cc923f1ccc ("tasklet: Introduce new initialization API")' > > introduced a new tasklet initialization API. This series converts > > all the scsi drivers to use the new tasklet_setup() API > > I've got to say I agree with Jens, this was a silly obfuscation: > > +#define from_tasklet(var, callback_tasklet, tasklet_fieldname) \ > + container_of(callback_tasklet, typeof(*var), tasklet_fieldname) > > Just use container_of directly since we all understand what it does. But then the lines get really long, wrapped, etc. This is what the timer_struct conversion did too (added a container_of wrapper), so I think it makes sense here too.
On Mon, 2020-08-17 at 12:28 -0700, Kees Cook wrote: > On Mon, Aug 17, 2020 at 07:41:58AM -0700, James Bottomley wrote: > > On Mon, 2020-08-17 at 14:24 +0530, Allen Pais wrote: > > > From: Allen Pais <allen.lkml@gmail.com> > > > > > > Commit 12cc923f1ccc ("tasklet: Introduce new initialization > > > API")' introduced a new tasklet initialization API. This series > > > converts all the scsi drivers to use the new tasklet_setup() API > > > > I've got to say I agree with Jens, this was a silly obfuscation: > > > > +#define from_tasklet(var, callback_tasklet, tasklet_fieldname) \ > > + container_of(callback_tasklet, typeof(*var), > > tasklet_fieldname) > > > > Just use container_of directly since we all understand what it > > does. > > But then the lines get really long, wrapped, etc. I really don't think that's a problem but if you want to add a new generic container_of that does typeof instead of insisting on the type, I'd be sort of OK with that ... provided you don't gratuitously alter the argument order. The thing I object to is that this encourages everyone to roll their own unnecessary container_of type macros in spite of the fact that it's function is wholly generic. It's fine if you're eliminating one of the arguments, or actually making the macro specific to the type, but in this case you're not, you're making a completely generic macro where the name is the only thing that's specific to this case. > This is what the timer_struct conversion did too (added a > container_of wrapper), so I think it makes sense here too. I didn't see that one to object to it ... James
> > > > > > > > Commit 12cc923f1ccc ("tasklet: Introduce new initialization > > > > API")' introduced a new tasklet initialization API. This series > > > > converts all the scsi drivers to use the new tasklet_setup() API > > > > > > I've got to say I agree with Jens, this was a silly obfuscation: > > > > > > +#define from_tasklet(var, callback_tasklet, tasklet_fieldname) \ > > > + container_of(callback_tasklet, typeof(*var), > > > tasklet_fieldname) > > > > > > Just use container_of directly since we all understand what it > > > does. > > > > But then the lines get really long, wrapped, etc. > > I really don't think that's a problem but if you want to add a new > generic container_of that does typeof instead of insisting on the type, > I'd be sort of OK with that ... provided you don't gratuitously alter > the argument order. > > The thing I object to is that this encourages everyone to roll their > own unnecessary container_of type macros in spite of the fact that it's > function is wholly generic. It's fine if you're eliminating one of the > arguments, or actually making the macro specific to the type, but in > this case you're not, you're making a completely generic macro where > the name is the only thing that's specific to this case. > > > This is what the timer_struct conversion did too (added a > > container_of wrapper), so I think it makes sense here too. > > I didn't see that one to object to it ... Since we could not get the generic API accepted, can I send out V2 which would use container_of()? Thanks,
From: Allen Pais <allen.lkml@gmail.com> Commit 12cc923f1ccc ("tasklet: Introduce new initialization API")' introduced a new tasklet initialization API. This series converts all the scsi drivers to use the new tasklet_setup() API Allen Pais (8): scsi: aic94xx: convert tasklets to use new tasklet_setup() API scsi: esas2r: convert tasklets to use new tasklet_setup() API scsi: ibmvscsi: convert tasklets to use new tasklet_setup() API scsi: isci: convert tasklets to use new tasklet_setup() API scsi: megaraid: convert tasklets to use new tasklet_setup() API scsi: mvsas: convert tasklets to use new tasklet_setup() API scsi: pm8001: convert tasklets to use new tasklet_setup() API scsi: pmcraid: convert tasklets to use new tasklet_setup() API drivers/scsi/aic94xx/aic94xx_hwi.c | 9 ++-- drivers/scsi/esas2r/esas2r.h | 2 +- drivers/scsi/esas2r/esas2r_init.c | 4 +- drivers/scsi/esas2r/esas2r_main.c | 4 +- drivers/scsi/ibmvscsi/ibmvfc.c | 6 +-- drivers/scsi/ibmvscsi/ibmvscsi.c | 8 +-- drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 7 ++- drivers/scsi/isci/host.c | 4 +- drivers/scsi/isci/host.h | 2 +- drivers/scsi/isci/init.c | 3 +- drivers/scsi/megaraid/megaraid_mbox.c | 9 ++-- drivers/scsi/megaraid/megaraid_sas.h | 2 +- drivers/scsi/megaraid/megaraid_sas_base.c | 16 +++--- drivers/scsi/megaraid/megaraid_sas_fusion.c | 14 +++--- drivers/scsi/mvsas/mv_init.c | 8 +-- drivers/scsi/pm8001/pm8001_init.c | 55 ++++++++++++--------- drivers/scsi/pm8001/pm8001_sas.h | 6 ++- drivers/scsi/pmcraid.c | 29 +++++------ drivers/scsi/pmcraid.h | 9 +++- 19 files changed, 104 insertions(+), 93 deletions(-)