> From: Jens Axboe [mailto:axboe@xxxxxxxxx] > On Thu, Sep 21 2006, David Somayajulu wrote: > > > From: Jens Axboe [mailto:axboe@xxxxxxxxx] > > > Sent: Thursday, September 21, 2006 11:48 AM > > > On Thu, Sep 21 2006, Jens Axboe wrote: > > > > On Thu, Sep 21 2006, Mike Christie wrote: > > > > > Jens Axboe wrote: > > > > > > On Thu, Sep 21 2006, Mike Christie wrote: > > > > > >> James Smart wrote: > > > > > >>> Why does your LLD need to reach up into the block layer to > > find an i/o ? > > > > > >>> > > > > > >>> Even if you are using tags as an index into something... I > > would think that > > > > > >>> having to go up to the blk layer to retrieve a something that > > should have > > > > > >>> already been known lower in the driver leaves room for race > > conditions. > > > > > >>> > > > > > >> This is how the blk and scsi tag api work for queue based > > tagging. We > > > > > >> have the scsi_find_tag() function which takes a scsi device and > > than > > > > > >> calls blk_queue_find_tag to get the request. It then does all > > the magic > > > > > >> sdev to request_queue and request to scsi command work and pass > > the LLD > > > > > >> the scsi command for the tag. So we can either add a driver > > array and do > > > > > >> some tag to scsi command or driver stucture mapping or we can > > use the > > > > > >> array already created in the scsi host block queue tag. And as > > you see > > > > > >> Dave's patch did the latter in the spirit of not duplicating > > what > > > > > >> scsi-ml or the block layer already do. > > > > > >> > > > > > >> I think there are some basic races though. For example, > > scsi_request_fn > > > > > >> calls blk_queue_start_tag with only the queue lock held and so > > if the > > > > > >> request_fn was called for two devices on the same host at the > > same time > > > > > >> they both could call find_first_zero_bit on the shared > > bqt->tag_map and > > > > > >> end up getting the same tag. > > > > > > > > > > > > Hrmpf good point, I suspect that would be easy enough to fix > > with just > > > > > > using a > > > > > > > > > > > > do { > > > > > > tag = ffz_bit(..); > > > > > > } while (test_and_set_bit(tag, map); > > > > > > > > > > > > construct. Agree? > > > > > > > > > > > > > > > > I think so. I think this is similar to what Dave was going to do > > too. > > > > > > > > Already checked in such a fix: > > > > > > > > > > http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=7ddbe6863ac4b7a55 > > 88aa4a70f262c098f3c469a > > > > Thanks. This a lot elegant than the one I came up with using a lock. > > Jens, any comments on this patch > > [RFC] [PATCH] helper function for retrieving scsi_cmd given host based > > block layer tag > > http://marc.theaimsgroup.com/?l=linux-scsi&m=115871027627964&w=2 > > Patch looks very nice to me, I'll apply it and let James scream if he > dislikes it for some reason (I don't see why, though). James and Christoph, I would you really appreciate if you could let me know where we stand on the inclusion of qla4xxx driver upstream. Block layer tagging was the last gating item in the last round and it has been taken care of. Thanks David Somayajulu - 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
