On 08/24/2010 10:06 PM, Alan Stern wrote:
quoted text
> On Tue, 24 Aug 2010, Jens Axboe wrote:
> 
>> On 2010-08-24 16:42, Alan Stern wrote:
>>>> Sounds like all you need is a way to return BLKPREP_DEFER_AND_STOP and
>>>> have the block layer stop the queue for you. When you need to restart,
>>>> you would insert a special request at the head of the queue and call
>>>> blk_start_queue() to get things going again.
>>>
>>> Yes.
>>>
>>> Suppose the driver needs to send two of these special requests before
>>> going back to normal operation.  Won't restarting the queue for the
>>> first special request also cause the following regular request to be
>>> passed to the driver before the second special request can be inserted?  
>>> Of course, the driver could cope with this simply by returning another
>>> BLKPREP_DEFER_AND_STOP.
>>
>> For that special request, you are sure to have some ->end_io() hook to
>> know when it's complete. When that triggers, you queue the 2nd special
>> request. And so on, for how many you need.
> 
> That's not what I meant.  Suppose the driver wants to carry out special
> requests A and B before carrying out request R, which is initially at
> the head of the queue.  The driver inserts A at the front, calls
> blk_start_queue(), and inserts B at the front when A completes.  
> What's to prevent the block layer from sending R to the driver while A
> is running?

Nothing, you will have to maintain that state and defer when
appropriate. Which should happen automatically, since you would not be
switching your state to running until request B has completed anyway.

quoted text
>>>> The only missing bit would then be the idle detection. That would need
>>>> to be in the block layer itself, and the scheme I described should be
>>>> fine for that still.
>>>
>>> Are you sure it needs to be in the block layer?  Is there no way for 
>>> the driver's completion handler to tell whether the queue is now empty?  
>>> Certainly it already has enough information to know whether the device 
>>> is still busy processing another request.  When the device is no longer 
>>> busy and the queue is empty, that's when the idle timer should be 
>>> started or restarted.
>>
>> To some extent there is, but there can be context outside of the queue
>> it doesn't know about. That is the case for the plugging rework, for
>> instance. That also removes the queue_empty() call. Then there's
>> blk_fetch_request(), but that may return NULL while there's IO pending
>> in the block layer - so not reliable for that either. The block layer is
>> tracking this state anyway, if you are leaving it to the driver then it
>> would have to check everytime it completes the last request it has. It's
>> cheaper to do in the block layer.
> 
> I see.  You're suggesting we add a new "power_mode" or "queue_idle"  
> callback to the request_queue struct, and make the block layer invoke
> this callback whenever a request completes and there are no other
> requests pending or in flight.  Right?  And similarly, invoke the
> callback (with a different argument) when the first request gets added
> to an otherwise empty queue.
> 
> That would suit my needs.

Yep, that is what I'm suggesting.

-- 
Jens Axboe

--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/