On Thu, 2010-09-02 at 10:24 -0700, Vasu Dev wrote: > 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. Hmmm, fair point here.. It is just something about seeing single block conditional with unlock and locking spins that makes me a little un-easy.. ;-) > > > > 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. Sure, I am thinking about these simple host_lock wrappers as more of a transitional look for LLDs more than anything.. Btw, I would be happy to include your forthcoming v2 patch into a lio-core-2.6.git branch, and give it some testing in the next week. Thanks! --nab -- 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
