Re: [PATCH][5/?] count writeback pages in nr_scanned

[Nick Piggin <nickpiggin@yahoo.com.au>]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

Andrew Morton wrote:
> Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> 
>> > If the queue is not congested, blk_congestion_wait() will still sleep.  See
>> > freed_request().
>> > 
>>
>> Hmm... doesn't look like it to me:
>>
>>          if (rl->count[rw] < queue_congestion_off_threshold(q))
>>                  clear_queue_congested(q, rw);
>>
>> And clear_queue_congested does an unconditional wakeup (if there
>> is someone sleeping on the congestion queue).
> 
> 
> That's my point.  blk_congestion_wait() will always sleep, regardless of
> the queue's congestion state.
> 

Oh yes, but it will return as soon as a single request is finished.
Which is probably a couple of milliseconds, rather than the 100 we
had hoped for. So the allocators will wake up again and go around
the loop and still make no progress.

However, if you had a plain io_schedule_timeout there, at least you
would sleep for the full extend of the specified timeout.

BTW. Jens, now that I look at freed_request, is the memory barrier
required? If so, what is it protecting?


[-- Attachment #2: blk-no-mb.patch --]
[-- Type: text/plain, Size: 1275 bytes --]



This memory barrier is not needed because the waitqueue will only get
waiters on it in the following situations:

rq->count has exceeded the threshold - however all manipulations of ->count
are performed under the runqueue lock, and so we will correctly pick up any
waiter.

Memory allocation for the request fails. In this case, there is no additional
help provided by the memory barrier. We are guaranteed to eventually wake
up waiters because the request allocation mempool guarantees that if the mem
allocation for a request fails, there must be some requests in flight. They
will wake up waiters when they are retired.



---

 linux-2.6-npiggin/drivers/block/ll_rw_blk.c |    1 -
 1 files changed, 1 deletion(-)

diff -puN drivers/block/ll_rw_blk.c~blk-no-mb drivers/block/ll_rw_blk.c
--- linux-2.6/drivers/block/ll_rw_blk.c~blk-no-mb	2005-01-06 15:37:05.000000000 +1100
+++ linux-2.6-npiggin/drivers/block/ll_rw_blk.c	2005-01-06 15:37:12.000000000 +1100
@@ -1630,7 +1630,6 @@ static void freed_request(request_queue_
 	if (rl->count[rw] < queue_congestion_off_threshold(q))
 		clear_queue_congested(q, rw);
 	if (rl->count[rw]+1 <= q->nr_requests) {
-		smp_mb();
 		if (waitqueue_active(&rl->wait[rw]))
 			wake_up(&rl->wait[rw]);
 		blk_clear_queue_full(q, rw);

_