Messages in this thread | | | Date | Tue, 28 Apr 2015 18:43:29 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2] ipc/mqueue: remove STATE_PENDING |
| |
On Tue, Apr 28, 2015 at 09:36:50AM -0700, Davidlohr Bueso wrote: > On Tue, 2015-04-28 at 14:37 +0200, Peter Zijlstra wrote: > > On Mon, Apr 27, 2015 at 08:24:53PM -0700, Davidlohr Bueso wrote: > > > +static inline void pipelined_send(struct wake_q_head *wake_q, > > > + struct mqueue_inode_info *info, > > > struct msg_msg *message, > > > struct ext_wait_queue *receiver) > > > { > > > receiver->msg = message; > > > list_del(&receiver->list); > > > + wake_q_add(wake_q, receiver->task); > > > + /* > > > + * Ensure that updating receiver->state is the last > > > + * write operation: As once set, the receiver can continue, > > > + * and if we don't have the reference count from the wake_q, > > > + * yet, at that point we can later have a use-after-free > > > + * condition and bogus wakeup. > > > + */ > > > + smp_wmb(); /* pairs with smp_rmb() in wq_sleep */ > > > > You have this barrier because we cannot rely on a failed cmpxchg() > > actually being a full barrier, right? > > Failed cmpxchg() calls implies that the task is never added to the queue > (duplicate, which I cannot see occurring in this patch), so nothing > wrong with the bogus wakeups mentioned in the comment. > > This barrier is not added by this patch though. Currently we have it > serializing with the wake_up_process() with STATE_READY, for similar > reasons. Because there is no task refcounting going on, the task can > easily disappear underneath us if the state is set before the wakeup. I > applied the same judgment here.
Well, if you can 'guarantee' the cmpxchg will not fail, you can then rely on the fact that cmpxchg implies a full barrier, which would obviate the need for the wmb.
| |