If we don't need precise timeout, we can have one single timer running at regular intervals, say every second. We also record the exact number of requests that are supposed to be done in a particular interval. This is done by computing the 'interval index' of a request, incrementing it when we send the request and decrementing the same when the request is completed. Timer can detect if all requests are done for that interval by looking at the 'interval number' In this method, timer can detect a timeout but doesn't know which command has timed out though. Aborting all pending requests is one option. As we don't start timer for each request, there is no need to stop any timer when the request is done. Thanks, Malahal. James Bottomley [James.Bottomley@xxxxxxxxxxxx] wrote: > On Tue, 2007-10-09 at 14:15 +0200, Jens Axboe wrote: > > On Tue, Oct 09 2007, Matthew Wilcox wrote: > > > On Mon, Oct 08, 2007 at 10:36:10PM -0700, malahal@xxxxxxxxxx wrote: > > > > Thank you Randy, Jens for your suggestions. I folded the second patch as > > > > it is just a clean up. Here is the fixed one patch version. > > > > > > I was thinking about this (in the context of shrinking scsi_cmnd -- > > > obviously, things are not improved if we simply move the timer to request > > > instead of scsi_cmnd). Why do we need a timer _per request_? We don't > > > need one per network packet. I appreciate we had one per scsi_cmnd and > > > this patch is just moving it upwards in the hierarchy, but perhaps we > > > can do better. > > > > > > What if we have one timer per request queue instead? It needs to expire > > > as soon as the earliest request timer would expire, then needs to be > > > reset to the next earliest one. We might walk the request queue more > > > frequently, but we'd save 48 bytes in the struct request. > > > > I agree, adding a full timer to each request is not nice. You jump over > > the actual implementation details of having just one timer in the queue > > though, it's pretty cheap to just say it can be done :-). You need to > > track each request anyways. If all drivers used the block layer tagging > > it would be easy since we are tracking each pending request in that > > case, but right now they don't. So pending requests may very well be > > outside of block layer knowledge. > > Can't we handle this a bit like the Linux timer infrastructure? Instead > of a timer per cmnd we have one per queue that's reset by commands > returning? If we retained a linked list of commands in timer order and > expiry times, that's still going to save us an unsigned long and two > pointers over struct timer_list. > > > You'd also end up using lots of extra cycles to find and move the timer > > when it expires (not likely) or is deleted (most likely). I'd greatly > > prefer the space overhead in this case, even if it is quite costly. > > If there's another command on the linked list, you mod_timer otherwise > you del_timer_sync. Returning commands obviously delete from this list. > > > There's also the issue of fiddling with just one vs many timers, > > potentially more cache bouncy. > > Yes, I agree it's more fiddly. > > The beauty is that I think it can all be done at the block layer via > helpers ... so it becomes Somebody Else's Problem from the SCSI point of > view ... > > 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
