Re: [RFC PATCH 2/5] block: Do not stop draining if waitqueue is not empty.

[Asias He <asias@redhat.com>]

On 05/21/2012 11:39 PM, Tejun Heo wrote:
> On Mon, May 21, 2012 at 05:08:30PM +0800, Asias He wrote:
>> If there are processes still in the wait queue, keep draining,
>> otherwise these processes would be in D state forever.
>>
>> I noticed this situation:
>> q->rq.count[0] == 0, q->rq.count[1] == 0, however wait queue
>> q->rq.wait[0].task_list and q->rq.wait[1].task_list are not empty.
>>
>> Cc: Jens Axboe<axboe@kernel.dk>
>> Cc: Tejun Heo<tj@kernel.org>
>> Cc: linux-fsdevel@vger.kernel.org
>> Signed-off-by: Asias He<asias@redhat.com>
>> ---
>>   block/blk-core.c |    1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index ca42fd7..2c2b585 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -394,6 +394,7 @@ void blk_drain_queue(struct request_queue *q, bool drain_all)
>>   				drain |= q->rq.count[i];
>>   				drain |= q->in_flight[i];
>>   				drain |= !list_empty(&q->flush_queue[i]);
>> +				drain |= waitqueue_active(&q->rq.wait[i]);
>
> Hmm... how does that happen?  What do you mean "you noticed this
> situation"?  Did that actually happen or you noticed such scenarios
> would be possible?  If rl.wait[] isn't empty with zero rq count, the
> queue would hang, so we should be fixing that situation instead of
> working around it from cleanup_queue.

Hi, Tejun

I actually saw this happened though it should not happen. I have no idea 
why this happens. Maybe unbalanced prepare_to_wait_exclusive() in 
get_request_wait() and wake_up() in __freed_request()?

With this happened, I saw some fio threads in D state which are sleeping 
on get_request_wait(). If I wake up the threads in the wait queue in 
q->abort_queue_fn() callback which i proposed in the 1/5 of this patch 
set, the queue cleanup and thus hot-unplug went pretty well. (Passed 
3000~ rounds of test, without this 2~ round of test would fail). See 
this patch [RFC PATCH 4/5] virtio-blk: Use q->abort_queue_fn() to abort.

-- 
Asias