On 09/17, Dmitry Vyukov wrote: > > I can update the patch description, but let me explain it here first.
Yes thanks. > Here is the essence of what happens: Aha, so you really meant that 2 put_pid's can race with each other, > // thread 1 > 1: pid->foo = 1; // foo is the first word of pid object > // then it does put_pid > 2: atomic_dec_and_test(&pid->count) // decrements count to 1 and > returns false so the function returns > > // thread 2 > // executes put_pid > 3: atomic_load(&pid->count); // returns 1, so proceed to kmem_cache_free > // then kmem_cache_free does: > 5: head->freelist = (void*)pid; > > This can be executed as: > > 4: *(void**)pid = head->freelist; > 1: pid->foo = 1; // foo is the first word of pid object > 2: atomic_dec_and_test(&pid->count) // decrements count to 1 and > returns false so the function returns > 3: atomic_load(&pid->count); // returns 1, so proceed to kmem_cache_free > 5: head->freelist = (void*)pid; Unless I am totally confused, everything is simpler. We can forget about the hoisting, freelist, etc. Thread 2 can see the result of atomic_dec_and_test(), but not the result of "pid->foo = 1". In this case in can free the object which can be re-allocated _before_ STORE(pid->foo) completes. Of course, this would be really bad. I need to recheck, but afaics this is not possible. This optimization is fine, but probably needs a comment. We rely on delayed_put_pid() called by RCU. And note that nobody can write to this pid after it is removed from the rcu-protected list. So I think this is false alarm, but I'll try to recheck tomorrow, it is too late for me today. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
