On Wed, 2010-09-01 at 14:38 -0700, Nicholas A. Bellinger wrote: > > Maybe two additional checks here is not so neat but not too bad > either > > as just two additional checks here, I excluded this unlocked_qcmd > checks > > from scsi_error queuecommand calling to not clutter its code there > with > > these additional checks w/o any good case for that code path. > > > > Hmmmm, handing off the locking like this between ML and LLD gets ugly > pretty quick, so I am not sure exactly sure if this is the right way > to > go about it.. > > Mabye in code terms we might need to consider converting some (or > all..?) struct Scsi_Host->host_lock accesses to some form of > Linux/SCSI > Host specific lock and unlock wrapper that is aware of the current > ->queuecommand() LLD lock status..? Obviously this would only need to > cover the queuecommand path here first, but perhaps would be useful in > other areas as well..? > This patch would add only two conditional locking and unlocking of host lock in scsi_dispatch_cmd and these are not applicable to all other several places host_lock use, therefore any wrapper for just these two new places is not worthy as that would hide conditions inside wrapper, or may be I'm missing something here. > This would at least (I think) allow ML and LLD code to be a tad bit > cleaner than something like the example above where > host->unlocked_qcmds > TRUE/FALSE conditionals exist directly within ML and LLD code. > Yeah but direct use makes things more obvious at first look. However neatness is worthy using wrapper(macros) if there are several such places. Anycase this is very minor code style thing here and I'm fine with wrapper if you want. Thanks Vasu -- 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
