lkml.org 
[lkml]   [2015]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH 5/5] percpu-rwsem: Optimize readers and reduce global impact
    Sorry for delay, finally I found the time to read this series...
    The code matches our previous discussion and I believe it is correct.

    Reviewed-by: Oleg Nesterov <oleg@redhat.com>


    Just one nit below,

    On 05/26, Peter Zijlstra wrote:
    >
    > struct percpu_rw_semaphore {
    > - unsigned int __percpu *fast_read_ctr;
    > - atomic_t write_ctr;
    > + unsigned int __percpu *refcount;
    > + int state;

    ....

    > +enum { readers_slow, readers_block };

    Now that we rely on rss_sync() and thus we do not have "readers_fast",
    I think that "bool reader_should_block" will look better.

    > +void percpu_down_write(struct percpu_rw_semaphore *sem)
    > {
    ...

    so it does

    rcu_sync_enter(&sem->rss);

    state = BLOCK;

    mb();

    wait_event(sem->writer, readers_active_check(sem));

    and this looks correct.

    The only nontrivial thing we need to ensure is that
    per_cpu_sum(*sem->refcount) == 0 can't be false positive. False
    negative is fine.

    And this means that if we see the result of this_cpu_dec() we must
    not miss the result of the previous this_cpu_inc() on another CPU.
    same or _another_ CPU.

    And this is true because if the reader does dec() on another CPU
    it does up_read() and this is only possible if down_read() didn't
    see state == BLOCK.

    But if it didn't see state == BLOCK then the writer must see the
    result of the previous down_read()->inc().

    IOW, we just rely on STORE-MB-LOAD, just the writer does LOAD
    multiple times in per_cpu_sum():

    DOWN_WRITE: DOWN_READ on CPU X:

    state = BLOCK; refcount[X]++;

    mb(); mb();

    sum = 0; if (state != BLOCK)
    sum += refcount[0]; return; /* success* /
    sum += refcount[1];
    ... refcount[X]--;
    sum += refcount[NR_CPUS];


    If the reader wins and takes the lock, then its addition to
    refcount[X] must be accounted by the writer.

    The writer can obviously miss dec() from the reader, but we rely
    on wake_up/wait_event in this case.

    Oleg.



    \
     
     \ /
      Last update: 2015-05-29 22:01    [W:4.031 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site