Re: [PATCH scsi-misc-2.6 01/05] scsi: make blk layer set REQ_SOFTBARRIER when a request is dispatched

[Tejun Heo <htejun@gmail.com>]

Nick Piggin wrote:
> On Wed, 2005-04-20 at 16:40 +0900, Tejun Heo wrote:
> 
>> Hello, Jens.
>>
>>On Wed, Apr 20, 2005 at 08:30:10AM +0200, Jens Axboe wrote:
>>
>>>Do it on requeue, please - not on the initial spotting of the request.
>>
>> This is the reworked version of the patch.  It sets REQ_SOFTBARRIER
>>in two places - in elv_next_request() on BLKPREP_DEFER and in
>>blk_requeue_request().
>>
>> Other patches apply cleanly with this patch or the original one and
>>the end result is the same, so take your pick.  :-)
>>
> 
> 
> I'm not sure that you need *either* one.
> 
> As far as I'm aware, REQ_SOFTBARRIER is used when feeding requests
> into the top of the block layer, and is used to guarantee the device
> driver gets the requests in a specific ordering.
> 
> When dealing with the requests at the other end (ie.
> elevator_next_req_fn, blk_requeue_request), then ordering does not
> change.
> 
> That is - if you call elevator_next_req_fn and don't dequeue the
> request, then that's the same request you'll get next time.
> 
> And blk_requeue_request will push the request back onto the end of
> the queue in a LIFO manner.
> 
> So I think adding barriers, apart from not doing anything, confuses
> the issue because it suggests there *could* be reordering without
> them.
> 
> Or am I completely wrong? It's been a while since I last got into
> the code.

 Well, yeah, all schedulers have dispatch queue (noop has only the
dispatch queue) and use them to defer/requeue, so no reordering will
happen, but I'm not sure they are required to be like this or just
happen to be implemented so.

 Hmm, well, it seems that setting REQ_SOFTBARRIER on requeue path isn't
necessary as we have INSERT_FRONT policy on requeue, and if
elv_next_req_fn() is required to return the same request when the
request isn't dequeued, you're right and we don't need this patch at
all.  We are guaranteed that all requeued requests are served in LIFO
manner.

 BTW, the same un-dequeued request rule is sort of already broken as
INSERT_FRONT request passes a returned but un-dequeued request, but,
then again, we need this behavior as we have to favor fully-prepped
requests over partially-prepped one.

-- 
tejun