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:3.084 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site