Patch added to scsi-rc-fixes-2.6: [SCSI] host lock push-down
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 12 lines · James Bottomley
Your commit:
[SCSI] host lock push-down
Move the mid-layer's ->queuecommand() invocation from being locked with the host lock to being unlocked to facilitate speeding up the critical path for drivers who don't need this lock taken anyway.
The patch below presents a simple SCSI host lock push-down as an equivalent transformation. No locking or other behavior should change with this patch. All existing bugs and locking orders are preserved.
Quoted 8 lines
Minimal code disturbance was attempted with this change. Most drivers needed only two one-line modifications for their host lock push-down.
Signed-off-by Jeff Garzik<[email protected]>Signed-off-by James Bottomley<[email protected]>has been added to the upstream SCSI tree
You can find it here:
No comments on renaming ->queuecommand to something else?
The consequences are rather dire if this goes unnoticed, yes?
Jeff
Quoted 7 lines · In http://marc.info/?l=linux-ide&m=128891665713984&w=2 I wrote:
An alternate arrangement, not presented by this patch, might be preferred: in order to make it clear that queuecommand locking has changed, one could s/queuecommand/queuecommand_nl/ in
Scsi_Host_Template, in order to guarantee that drivers are either
(a) upgraded or (b) broken at compile time. Compile-time detection of new locking may be desirable, and I'll volunteer to change my patch to do that, if community members prefer that route instead of below.
Quoted 16 lines · Jeff Garzik
Quoted 15 lines · James Bottomley
Your commit:
[SCSI] host lock push-down
Move the mid-layer's ->queuecommand() invocation from being locked with the host lock to being unlocked to facilitate speeding up the critical path for drivers who don't need this lock taken anyway.
The patch below presents a simple SCSI host lock push-down as an equivalent transformation. No locking or other behavior should change with this patch. All existing bugs and locking orders are preserved.
Minimal code disturbance was attempted with this change. Most drivers needed only two one-line modifications for their host lock push-down.
Quoted 7 lines
Quoted 5 lines
Signed-off-by Jeff Garzik<[email protected]>Signed-off-by James Bottomley<[email protected]>has been added to the upstream SCSI tree
You can find it here:No comments on renaming ->queuecommand to something else?
What we wondered about doing differently isn't really relevant for a change log ... that should just really be about what was done (to avoid confusion).
Quoted 1 line
The consequences are rather dire if this goes unnoticed, yes?
You mean if there's a missed in-tree driver? Yes, but I took care to make sure all SCSI drivers were accounted for. For out of tree drivers, as with the eh lock push down, it's caveat emptor.
James
Quoted 20 lines · James Bottomley
Quoted 19 lines · Jeff Garzik
Quoted 18 lines · James Bottomley
Your commit:
[SCSI] host lock push-down
Move the mid-layer's ->queuecommand() invocation from being locked with the host lock to being unlocked to facilitate speeding up the critical path for drivers who don't need this lock taken anyway.
The patch below presents a simple SCSI host lock push-down as an equivalent transformation. No locking or other behavior should change with this patch. All existing bugs and locking orders are preserved.
Minimal code disturbance was attempted with this change. Most drivers needed only two one-line modifications for their host lock push-down.
Signed-off-by Jeff Garzik<[email protected]>Signed-off-by James Bottomley<[email protected]>
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
has been added to the upstream SCSI tree
You can find it here:No comments on renaming ->queuecommand to something else?
What we wondered about doing differently isn't really relevant for a change log ... that should just really be about what was done (to avoid confusion).
Wasn't referring to the changelog (perhaps shouldn't have quoted that); just asking the question generally.
Quoted 5 lines
Quoted 1 line
The consequences are rather dire if this goes unnoticed, yes?
You mean if there's a missed in-tree driver? Yes, but I took care to make sure all SCSI drivers were accounted for. For out of tree drivers, as with the eh lock push down, it's caveat emptor.
Thinking about out-of-tree drivers, yes.
Jeff
Quoted 34 lines · Jeff Garzik
Quoted 20 lines · James Bottomley
Quoted 19 lines · Jeff Garzik
Quoted 18 lines · James Bottomley
Your commit:
[SCSI] host lock push-down
Move the mid-layer's ->queuecommand() invocation from being locked with the host lock to being unlocked to facilitate speeding up the critical path for drivers who don't need this lock taken anyway.
The patch below presents a simple SCSI host lock push-down as an equivalent transformation. No locking or other behavior should change with this patch. All existing bugs and locking orders are preserved.
Minimal code disturbance was attempted with this change. Most drivers needed only two one-line modifications for their host lock push-down.
Signed-off-by Jeff Garzik<[email protected]>Signed-off-by James Bottomley<[email protected]>Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
has been added to the upstream SCSI tree
You can find it here:No comments on renaming ->queuecommand to something else?
What we wondered about doing differently isn't really relevant for a change log ... that should just really be about what was done (to avoid confusion).
Wasn't referring to the changelog (perhaps shouldn't have quoted that); just asking the question generally.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
The consequences are rather dire if this goes unnoticed, yes?
You mean if there's a missed in-tree driver? Yes, but I took care to make sure all SCSI drivers were accounted for. For out of tree drivers, as with the eh lock push down, it's caveat emptor.
Thinking about out-of-tree drivers, yes.
Hi Jeff and James,
Thank you for getting this initial patch merged. I really think this
was and is the best choice moving forward. Also, a seriously big thank
you to all of the other folks who have helped identify LLDs issues for host_lock less mode for drivers!!
In the next days I will get a atomic_t scsi_host->cmd_serial_number patch rebased (which is really very minor at this point w/o the scsi_error.c changes), and merge the current host_lock-less 'scoreboard'
on top of jgarzik's code and tag for .38. So, please let me know if
you would to include minor the atomic_t scsi_host->cmd_serial_number patch for .37 or if you would rather have this immediately preceed the first series of "enable host-lock_less for LLD vendor superturbo hba" for the .38 round.
Best,
--nab