Messages in this thread | | | Date | Thu, 06 Jan 2005 19:16:44 +1100 | From | Nick Piggin <> | Subject | memory 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/
| |