Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-22 Thread Herbert Xu
Eric Dumazet wrote: > > What I said is : > > In @head you already have the correct nulls value, from hash table. > > You do not need to recompute this value, and/or test if hash table chain > is empty. > > If hash bucket is empty, it contains the appropriate NULLS value. > > If you are paranoi

Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-22 Thread Dmitry Vyukov
On Tue, Sep 22, 2015 at 10:19 AM, Thomas Graf wrote: > On 09/21/15 at 04:03pm, Eric Dumazet wrote: >> What I said is : >> >> In @head you already have the correct nulls value, from hash table. >> >> You do not need to recompute this value, and/or test if hash table chain >> is empty. >> >> If hash

Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-22 Thread Thomas Graf
On 09/21/15 at 04:03pm, Eric Dumazet wrote: > What I said is : > > In @head you already have the correct nulls value, from hash table. > > You do not need to recompute this value, and/or test if hash table chain > is empty. > > If hash bucket is empty, it contains the appropriate NULLS value. >

Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-21 Thread Eric Dumazet
On Tue, 2015-09-22 at 00:25 +0200, Thomas Graf wrote: > On 09/21/15 at 07:51am, Eric Dumazet wrote: > > The important part here is that we rehash an item, so we need to make > > sure to maintain consistent ->next field, and need to prevent compiler > > from using ->next as a temporary variable. > >

Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-21 Thread Thomas Graf
On 09/21/15 at 07:51am, Eric Dumazet wrote: > The important part here is that we rehash an item, so we need to make > sure to maintain consistent ->next field, and need to prevent compiler > from using ->next as a temporary variable. > > ptr->next = 1UL | ((base + offset) << 1); > > Is dangerous

Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-21 Thread Eric Dumazet
On Mon, 2015-09-21 at 17:10 +0200, Dmitry Vyukov wrote: > On Mon, Sep 21, 2015 at 4:51 PM, Eric Dumazet wrote: > > On Mon, 2015-09-21 at 06:31 -0700, Eric Dumazet wrote: > >> On Mon, 2015-09-21 at 10:08 +0200, Dmitry Vyukov wrote: > >> > rhashtable_rehash_one() uses plain writes to update entry->n

Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-21 Thread Dmitry Vyukov
On Mon, Sep 21, 2015 at 4:51 PM, Eric Dumazet wrote: > On Mon, 2015-09-21 at 06:31 -0700, Eric Dumazet wrote: >> On Mon, 2015-09-21 at 10:08 +0200, Dmitry Vyukov wrote: >> > rhashtable_rehash_one() uses plain writes to update entry->next, >> > while it is being concurrently accessed by readers. >>

Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-21 Thread Eric Dumazet
On Mon, 2015-09-21 at 06:31 -0700, Eric Dumazet wrote: > On Mon, 2015-09-21 at 10:08 +0200, Dmitry Vyukov wrote: > > rhashtable_rehash_one() uses plain writes to update entry->next, > > while it is being concurrently accessed by readers. > > Unfortunately, the compiler is within its rights to (for

Re: [PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-21 Thread Eric Dumazet
On Mon, 2015-09-21 at 10:08 +0200, Dmitry Vyukov wrote: > rhashtable_rehash_one() uses plain writes to update entry->next, > while it is being concurrently accessed by readers. > Unfortunately, the compiler is within its rights to (for example) use > byte-at-a-time writes to update the pointer, whi

[PATCH] lib: fix data race in rhashtable_rehash_one

2015-09-21 Thread Dmitry Vyukov
rhashtable_rehash_one() uses plain writes to update entry->next, while it is being concurrently accessed by readers. Unfortunately, the compiler is within its rights to (for example) use byte-at-a-time writes to update the pointer, which would fatally confuse concurrent readers. Use WRITE_ONCE to