On 10/28/2010 08:26 PM, Andi Kleen wrote: >> I disagree with your approach this introduces a spin_unlock_irqrestore >> call site at every return, in the usually huge queuecommand. > > I converted the full tree and in practice it turns out there > are very few returns in nearly all queuecommands. So it won't > make much difference. > "few returns" is too much. Any thing bigger than 1 is a total wast. And the mess?!? Where to add the flags? where the returns? Need a "{...}" or not. Lots of manual intervention, and possible errors. I bet with my approach you wouldn't need to manually fix a single file. > Longer term they will be all hopefully gone again anyways. > That one I'm most afraid of. These that did not get fixed in this round, will not be fixed for a long time (if ever). I'd even go anal and not open code the lock but actually call the original __queue_command through a MACRO, that can be change in one place. (And will solve your patchset bisect-ability) - XXX_queue_command(... + XXX_queue_command_unlocked(... + XXX_queue_command(... + { + return SCSI_LOCKED_QUEUECOMMAND(XXX_queue_command_unlocked, ...); + } > -Andi But since I'm only blabing, the one that "do", gets to decide ;-) . Perhaps next time. Boaz -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html