Messages in this thread | | | Date | Fri, 12 Nov 2010 21:03:07 -0500 | From | Jeff Garzik <> | Subject | Re: [GIT PULL] SCSI queuecommand API change for 2.6.37-rc1 |
| |
On 11/12/2010 08:42 PM, Linus Torvalds wrote: > On Fri, Nov 12, 2010 at 3:55 PM, James Bottomley > <James.Bottomley@suse.de> wrote: >> >> This patch set contains a single patch modifying the SCSI queuecommand >> host template API to go from being called with the host lock held to >> being called locklessly. The transformation is a directly equivalent >> one (i.e. the locking is simply pushed into each HBA) but will form the >> basis for optimising locking in the driver patch for the next merge >> window. > > Ok, so we talked about this patch at the KS, but I never saw it. > > And now seeing it, I do detest it. > > Why? Because if some driver gets missed for any reason (notably if > it's currently out of tree, and gets merged later), afaik there will > be ABSOLUTELY ZERO compiler warnings or anything about it, because you > kept the "queuecommand" function exactly the same. Whether it's a > locked or non-locked one, it always is of type [...] > So please: when you change the semantics of a function, just change > the function prototype (or function name) at the same time. Especially > when it comes to a driver interface, so that the drivers don't get > taken by surprise. > > Type safety and automatic compiler warnings really are our friends. > Especially when the patch was presumably mostly auto-generated, and > maybe the script missed something, and missing some conversion has > such subtle effects.
That is precisely what I mentioned in the original patch: http://marc.info/?l=linux-ide&m=128891665713984&w=2
And brought this up again when James merged my patch: http://marc.info/?l=linux-scsi&m=128943169802009&w=2
I even volunteer[ed] to do the work to make it happen...
Jeff
P.S. My patch was not auto-generated. One of the two previous attempts at this change was scripted w/ coccinelle, and it created obvious locking problems.
| |