Re: [PATCH 0/8] scsi: convert tasklets to use new tasklet_setup()

2020-09-01 Thread Allen
> > > >
> > > > 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,

-- 
   - Allen


Re: [PATCH 0/8] scsi: convert tasklets to use new tasklet_setup()

2020-08-17 Thread James Bottomley
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 
> > > 
> > > 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



Re: [PATCH 0/8] scsi: convert tasklets to use new tasklet_setup()

2020-08-17 Thread James Bottomley
On Mon, 2020-08-17 at 14:24 +0530, Allen Pais wrote:
> From: Allen Pais 
> 
> 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



Re: [PATCH 0/8] scsi: convert tasklets to use new tasklet_setup()

2020-08-17 Thread Kees Cook
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 
> > 
> > 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.

-- 
Kees Cook


[PATCH 0/8] scsi: convert tasklets to use new tasklet_setup()

2020-08-17 Thread Allen Pais
From: Allen Pais 

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(-)

-- 
2.17.1