lkml.org 
[lkml]   [2015]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH RFC tip/core/rcu 05/14] rcu: Abstract sequence counting from synchronize_sched_expedited()
    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



    \
     
     \ /
      Last update: 2015-07-02 16:41    [W:2.694 / U:0.348 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site