On Thu, Jul 02, 2015 at 10:50:41AM +0200, Peter Zijlstra wrote:
> On Wed, Jul 01, 2015 at 03:18:04PM -0700, Paul E. McKenney wrote:
> > On Wed, Jul 01, 2015 at 12:27:17PM +0200, Peter Zijlstra wrote:
>
> > > That wants to be an ACQUIRE, right?
> >
> > I cannot put the acquire in the WARN_ON_ONCE() because there
> > are configurations where WARN_ON_ONCE() is compiled out. I could
> > conditionally compile, but given that this is nothing like a fastpath,
> > I cannot really justify doing that.
>
> Fair enough.
>
> > We could define an smp_store_acquire(), but that would require a full
> > barrier against subsequent loads. The C++ committee hit this one when
> > trying to implement seqeunce locking using the C/C++11 atomics. ;-)
>
> Yeah, I'm not sure how much sense smp_store_acquire() makes, but I'm
> fairly sure this isn't the first time I've wondered about it.
>
> > > > +static bool rcu_seq_done(unsigned long *sp, unsigned long s)
> > > > +{
> > > > + return ULONG_CMP_GE(READ_ONCE(*sp), s);
> > >
> > > I'm always amused you're not wanting to rely on 2s complement for
> > > integer overflow. I _know_ its undefined behaviour in the C rule book,
> > > but the entire rest of the kernel hard assumes it.
> >
> > I take it you have never seen the demonic glow in the eyes of a compiler
> > implementer when thinking of all the code that can be broken^W^W^W^W^W
> > optimizations that are enabled by relying on undefined behavior for
> > signed integer overflow? ;-)
>
> Note that this is unsigned integers, but yes I know, you've said. But
> they cannot unilaterally change this 'undefined' behaviour because its
> been defined as 'whatever the hardware does' for such a long time.
For pure unsigned arithmetic, their options are indeed limited. For a
cast to signed, I am not so sure. I have been using time_before() and
friends for jiffy comparisons, which does a cast to signed after the
subtraction. Signed overflow is already unsafe with current compilers,
though the kernel suppresses these.
> Likewise they can dream all they want about breaking our concurrent code
> and state we should use the brand spanking new primitives, sod 30 years
> of existing code, but that's just not realistic either.
>
> Even if we didn't 'have' to support a wide range of compiler versions,
> most of which do not even support these new fangled primitives, who is
> going to audit our existing many million lines of code? Not to mention
> the many more million lines of code in other projects that rely on these
> same things.
>
> Its really time for them to stop wanking and stare reality in the face.
Indeed, I have been and will be continuing to make myself unpopular with
that topic. ;-)
> > > > +/* Wrapper functions for expedited grace periods. */
> > > > +static void rcu_exp_gp_seq_start(struct rcu_state *rsp)
> > > > +{
> > > > + rcu_seq_start(&rsp->expedited_sequence);
> > > > +}
> > > > +static void rcu_exp_gp_seq_end(struct rcu_state *rsp)
> > > > +{
> > > > + rcu_seq_end(&rsp->expedited_sequence);
> > > > +}
> > > > +static unsigned long rcu_exp_gp_seq_snap(struct rcu_state *rsp)
> > > > +{
> > > > + return rcu_seq_snap(&rsp->expedited_sequence);
> > > > +}
> > > > +static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s)
> > > > +{
> > > > + return rcu_seq_done(&rsp->expedited_sequence, s);
> > > > +}
> > >
> > > This is wrappers for wrappers sake? Why?
> >
> > For _rcu_barrier(), as noted in the commit log.
>
> Yes it said; but why? Surely _rcu_barrier() can do the
> ->expedited_sequence thing itself, that hardly seems worthy of a
> wrapper.
Ah, you want synchronize_rcu_expedited() and synchronize_sched_expedited()
to use rcu_seq_start() and friends directly. I can certainly do that.
Thanx, Paul
--
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/
