Hi James, Thanks for the input, I am rethinking my patch. I agree with your second and third points. However, I am not using tag_map for this feature: In blk_queue_start_tag(), I assumed q->tag_busy_list contains the list of requests send to given device when q is tagged. q is the one managed by scsi_request_fn(), allocated in scsi_alloc_sdev() by scsi_alloc_queue(); I believe it is unique per scsi device. q->tag_busy_list is not shared by several device, can't I use it to track requests send to a given disk? Let me know if I missed something. Thanks, Gwendal. On Tue, Mar 11, 2008 at 6:44 PM, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > On Tue, 2008-03-11 at 17:41 -0700, Gwendal Grignou wrote: > > Please ignore the patch I sent earlier, it was out of context. > > [scsi_sysfs: Fix cut and paste errors] > > This patch allows to throttle command queuing to a device, if > > older commands are outstanding for too long already. ... > > Firstly there's a structural problem with this: you don't take into > account the fact that the tag map may be shared, so the first entry in > the tag list may belong to a completely different device. > > Secondly, this req->timeout_time_thres is really at the wrong layer: > it's only ever set and read from SCSI, so there's not really a good > reason to place it in the request ... it should probably be in the > command. Additionally, cmnd->jiffies_at_alloc already exists, so you > can probably get it do do everything you need without adding even a > parameter to the command. > > Finally, it doesn't look like it will work well with error handling: we > use the ->queuecommand path for certain error handling commands; if the > device is stopped and we've passed the threshold, this will reject the > error handling commands as well. The basic problem here is that the tag > is held in the block layer and isn't released until the command is > completed (after error handling is done). > > James > > > -- 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
