Re: [PATCH 2/5] ipc/mqueue.c: Update/document memory barriers

From: Davidlohr Bueso
Date: Mon Oct 14 2019 - 02:30:38 EST


On Fri, 11 Oct 2019, Manfred Spraul wrote:
But you are right, there are two different scenarios:

1) thread already in another wake_q, wakeup happens immediately after the cmpxchg_relaxed().

This scenario is safe, due to the smp_mb__before_atomic() in wake_q_add()

2) thread woken up but e.g. a timeout, see ->state=STATE_READY, returns to user space, calls sys_exit.

This must not happen before get_task_struct acquired a reference.

And this appears to be unsafe: get_task_struct() is refcount_inc(), which is refcount_inc_checked(), which is according to lib/refcount.c fully unordered.

Thus: ->state=STATE_READY can execute before the refcount increase.

Thus: ->state=STATE_READY needs a smp_store_release(), correct?

What if we did the reference count explicitly, and then just use
wake_q_add_safe()? That would avoid the extra barrier, __pipelined_op()
would become:

list_del();
get_task_struct();
wake_q_add_safe();
WRITE_ONCE(->state, STATE_READY);

Thanks,
Davidlohr