lkml.org 
[lkml]   [2005]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
Subjectmemory barrier in ll_rw_blk.c (was Re: [PATCH][5/?] count writeback pages in nr_scanned)
Jens Axboe wrote:
> On Thu, Jan 06 2005, Nick Piggin wrote:

>>
>>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.
>
>
> Not sure I agree completely. Yes it will work, but only because it tests
> <= q->nr_requests and I don't think that 'eventually' is good enough :-)
>
> The actual waitqueue manipulation doesn't happen under the queue lock,
> so the memory barrier is needed to pickup the change on SMP. So I'd like
> to keep the barrier.
>

No that's right... but between the prepare_to_wait and the io_schedule,
get_request takes the lock and checks nr_requests. I think we are safe?

> I'd prefer to add smp_mb() to waitqueue_active() actually!
>

That may be a good idea (I haven't really taken much notice of how other
code uses it).

I'm not worried about any possible performance advantages of removing it,
rather just having a memory barrier without comments can be perplexing.
-
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/

\
 
 \ /
  Last update: 2005-03-22 14:09    [W:0.046 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site