From: Davidlohr Bueso <dave@stgolabs.net>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-kernel@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>,
tglx@linutronix.de, Manfred Spraul <manfred@colorfullife.com>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v4] ipc/msg: Implement lockless pipelined wakeups
Date: Thu, 21 Jul 2016 12:47:35 -0700 [thread overview]
Message-ID: <20160721194735.GA1881@linux-80c1.suse> (raw)
In-Reply-To: <20160721102356.GB6323@linutronix.de>
On Thu, 21 Jul 2016, Sebastian Andrzej Siewior wrote:
>* Davidlohr Bueso | 2016-07-20 17:16:12 [-0700]:
>
>>Just as with expunge_all and the E2BIG case, could you remove that explicit
>>barrier (B) and just rely on wake_q_add?
>
>Just did. So we have just a smp_rmb() on the reader side and the
>comment talks about smb_wmb() and at the spot where we should have the
>smb_wmb we have a comment why we don't have one :)
>For my understanding: we need that smp_rmb() to ensure that everything
>past that cmpxchg() is visible on all other CPUs so we don't have the
>wakeup before we r_msg reads != -EAGAIN, right?
Hmm I'm having second thoughts about the need for barrier (A). As you know,
originally we had it to prevent races with do_exit() from the receiver thread
if the r_msg was set before doing the wakeup, we could face a use-after-free
scenario.
Now, by delaying the wakeup, the receiver task should always see whatever r_msg
is set to by the waker, even if we get reordered with wake_q_add() as the
actual wakeup_process() does not occur yet, and hence the receiver is still
blocked while this is going on -- iow, we avoid entirely the need to explicitly
wait until pipelined_send/expunge_all are done. Similarly, "barrier (B)" simply
serves to pair with wake_up_q() such that we don't miss wakeups, but that's
always handled by the wake_q machinery anyway.
So if this is the case (which is how you had it in some previous version of this
patch), we can get rid of the pair diagram altogether as well. Manfred, Peter, does
all this make sense to you guys?
Thanks,
Davidlohr
prev parent reply other threads:[~2016-07-21 19:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-20 15:45 [PATCH v4] ipc/msg: Implement lockless pipelined wakeups Sebastian Andrzej Siewior
2016-07-21 0:16 ` Davidlohr Bueso
2016-07-21 10:07 ` [PATCH v5] " Sebastian Andrzej Siewior
2016-07-21 10:23 ` [PATCH v4] " Sebastian Andrzej Siewior
2016-07-21 19:47 ` Davidlohr Bueso [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160721194735.GA1881@linux-80c1.suse \
--to=dave@stgolabs.net \
--cc=akpm@linux-foundation.org \
--cc=bigeasy@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=manfred@colorfullife.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).