From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: umgwanakikbuti@gmail.com, mingo@elte.hu, ktkhai@parallels.com,
rostedt@goodmis.org, tglx@linutronix.de, juri.lelli@gmail.com,
pang.xunlei@linaro.org, oleg@redhat.com,
wanpeng.li@linux.intel.com, linux-kernel@vger.kernel.org,
Al Viro <viro@ZenIV.linux.org.uk>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 11/18] seqcount: Introduce raw_write_seqcount_barrier()
Date: Wed, 17 Jun 2015 07:57:12 -0700 [thread overview]
Message-ID: <20150617145712.GZ3913@linux.vnet.ibm.com> (raw)
In-Reply-To: <20150617122924.GP3644@twins.programming.kicks-ass.net>
On Wed, Jun 17, 2015 at 02:29:24PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 11, 2015 at 02:45:57PM -0700, Paul E. McKenney wrote:
> > Color me slow and stupid. Maybe due to reviewing a patch too early in
> > the morning, who knows?
> >
> > There is nothing above that prevents the compiler and the CPU from
> > reordering the assignments to X and Y with the increment of s->sequence++.
> > One fix would be as follows:
> >
> > static inline void raw_write_seqcount_barrier(seqcount_t *s)
> > {
> > smp_wmb();
> > s->sequence++;
> > smp_wmb();
> > s->sequence++;
> > smp_wmb();
> > }
> >
> > Of course, this assumes that the accesses surrounding the call to
> > raw_write_seqcount_barrier() are writes. If they can be a reads,
> > the two added smp_wmb() calls need to be full barriers.
>
> I have updated the Changelog to hopefully explain things better.
>
> I did leave off the READ/WRITE ONCE stuff, because I could not come up
> with a scenario where it makes a difference -- I appreciate paranoia,
> but I also think we should not overdo the thing.
I can only conclude that you have not read this document:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4455.html
Specifically, please keep in mind that unless you mark either the variable
or the memory access, the compiler is within its rights to assume that
there are no concurrent accesses to that variable. For but one example,
if you do a normal store to a given variable, then the compiler is
within its rights to use that variable as temporary storage prior to
that store. And yes, you can reasonably argue that no sane compiler
would store something else to s->sequence given that it could free up
a register by storing the incremented value, but the fact remains that
you have given it permission to do so if it wants.
Thanx, Paul
> ---
> Subject: seqcount: Introduce raw_write_seqcount_barrier()
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Thu Jun 11 12:35:48 CEST 2015
>
> Introduce raw_write_seqcount_barrier(), a new construct that can be
> used to provide write barrier semantics in seqcount read loops instead
> of the usual consistency guarantee.
>
> raw_write_seqcount_barier() is equivalent to:
>
> raw_write_seqcount_begin();
> raw_write_seqcount_end();
>
> But avoids issueing two back-to-back smp_wmb() instructions.
>
> This construct works because the read side will 'stall' when observing
> odd values. This means that -- referring to the example in the comment
> below -- even though there is no (matching) read barrier between the
> loads of X and Y, we cannot observe !x && !y, because:
>
> - if we observe Y == false we must observe the first sequence
> increment, which makes us loop, until
>
> - we observe !(seq & 1) -- the second sequence increment -- at which
> time we must also observe T == true.
>
> Cc: Al Viro <viro@ZenIV.linux.org.uk>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Paul McKenney <paulmck@linux.vnet.ibm.com>
> Suggested-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> include/linux/seqlock.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> --- a/include/linux/seqlock.h
> +++ b/include/linux/seqlock.h
> @@ -233,6 +233,47 @@ static inline void raw_write_seqcount_en
> s->sequence++;
> }
>
> +/**
> + * raw_write_seqcount_barrier - do a seq write barrier
> + * @s: pointer to seqcount_t
> + *
> + * This can be used to provide an ordering guarantee instead of the
> + * usual consistency guarantee. It is one wmb cheaper, because we can
> + * collapse the two back-to-back wmb()s.
> + *
> + * seqcount_t seq;
> + * bool X = true, Y = false;
> + *
> + * void read(void)
> + * {
> + * bool x, y;
> + *
> + * do {
> + * int s = read_seqcount_begin(&seq);
> + *
> + * x = X; y = Y;
> + *
> + * } while (read_seqcount_retry(&seq, s));
> + *
> + * BUG_ON(!x && !y);
> + * }
> + *
> + * void write(void)
> + * {
> + * Y = true;
> + *
> + * raw_write_seqcount_barrier(seq);
> + *
> + * X = false;
> + * }
> + */
> +static inline void raw_write_seqcount_barrier(seqcount_t *s)
> +{
> + s->sequence++;
> + smp_wmb();
> + s->sequence++;
> +}
> +
> /*
> * raw_write_seqcount_latch - redirect readers to even/odd copy
> * @s: pointer to seqcount_t
>
next prev parent reply other threads:[~2015-06-17 14:58 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-11 12:46 [PATCH 00/18] sched: balance callbacks v4 Peter Zijlstra
2015-06-11 12:46 ` [PATCH 01/18] sched: Replace post_schedule with a balance callback list Peter Zijlstra
2015-06-11 15:32 ` Kirill Tkhai
2015-06-18 23:00 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 02/18] sched: Use replace normalize_task() with __sched_setscheduler() Peter Zijlstra
2015-06-18 23:00 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 03/18] sched: Allow balance callbacks for check_class_changed() Peter Zijlstra
2015-06-18 23:01 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 04/18] sched,rt: Remove return value from pull_rt_task() Peter Zijlstra
2015-06-18 23:01 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 05/18] sched,rt: Convert switched_{from,to}_rt() / prio_changed_rt() to balance callbacks Peter Zijlstra
2015-06-18 23:01 ` [tip:sched/hrtimers] sched, rt: Convert switched_{from, to}_rt() " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 06/18] sched,dl: Remove return value from pull_dl_task() Peter Zijlstra
2015-06-18 23:02 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 07/18] sched,dl: Convert switched_{from,to}_dl() / prio_changed_dl() to balance callbacks Peter Zijlstra
2015-06-18 23:02 ` [tip:sched/hrtimers] sched, dl: Convert switched_{from, to}_dl() " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 08/18] hrtimer: Remove HRTIMER_STATE_MIGRATE Peter Zijlstra
2015-06-18 22:18 ` [tip:timers/core] " tip-bot for Oleg Nesterov
2015-06-11 12:46 ` [PATCH 09/18] hrtimer: Fix hrtimer_is_queued() hole Peter Zijlstra
2015-06-18 22:18 ` [tip:timers/core] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 10/18] seqcount: Rename write_seqcount_barrier() Peter Zijlstra
2015-06-18 22:19 ` [tip:timers/core] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 11/18] seqcount: Introduce raw_write_seqcount_barrier() Peter Zijlstra
2015-06-11 15:33 ` Paul E. McKenney
2015-06-11 21:45 ` Paul E. McKenney
2015-06-12 7:08 ` Peter Zijlstra
2015-06-12 18:59 ` Oleg Nesterov
2015-06-17 12:29 ` Peter Zijlstra
2015-06-17 14:57 ` Paul E. McKenney [this message]
2015-06-17 15:11 ` Peter Zijlstra
2015-06-17 15:42 ` Paul E. McKenney
2015-06-17 16:58 ` Peter Zijlstra
2015-06-17 15:49 ` Peter Zijlstra
2015-06-17 16:37 ` Paul E. McKenney
2015-06-17 17:11 ` Peter Zijlstra
2015-06-17 18:02 ` Paul E. McKenney
2015-06-18 9:15 ` Peter Zijlstra
2015-06-18 9:40 ` Ingo Molnar
2015-06-18 10:40 ` Peter Zijlstra
2015-06-18 16:54 ` Paul E. McKenney
2015-06-18 17:10 ` Steven Rostedt
2015-06-18 17:51 ` Paul E. McKenney
2015-06-18 22:19 ` [tip:timers/core] seqcount: Introduce raw_write_seqcount_barrier( ) tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 12/18] hrtimer: Allow hrtimer::function() to free the timer Peter Zijlstra
2015-06-18 22:19 ` [tip:timers/core] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 13/18] sched,dl: Fix sched class hopping CBS hole Peter Zijlstra
2015-06-18 23:02 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 14/18] sched: Move code around Peter Zijlstra
2015-06-18 23:02 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 15/18] sched: Streamline the task migration locking a little Peter Zijlstra
2015-06-18 23:03 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 16/18] lockdep: Simplify lock_release() Peter Zijlstra
2015-06-18 23:03 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 17/18] lockdep: Implement lock pinning Peter Zijlstra
2015-06-18 23:03 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-06-11 12:46 ` [PATCH 18/18] sched,lockdep: Employ " Peter Zijlstra
2015-06-18 23:04 ` [tip:sched/hrtimers] " tip-bot for Peter Zijlstra
2015-12-29 5:41 ` [PATCH 00/18] sched: balance callbacks v4 Byungchul Park
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=20150617145712.GZ3913@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.com \
--cc=juri.lelli@gmail.com \
--cc=ktkhai@parallels.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=oleg@redhat.com \
--cc=pang.xunlei@linaro.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=umgwanakikbuti@gmail.com \
--cc=viro@ZenIV.linux.org.uk \
--cc=wanpeng.li@linux.intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.