Date: Thu, 17 Nov 2016 12:31:34 -0800
From: Lance Roy <ldr709@gmail.com>
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: Boqun Feng <boqun.feng@gmail.com>,
        "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>,
        dipankar@in.ibm.com, akpm@linux-foundation.org,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        David Howells <dhowells@redhat.com>,
        Eric Dumazet <edumazet@google.com>, dvhart@linux.intel.com,
        =?UTF-8?B?RnLDqWTDqXJpYw==?= Weisbecker <fweisbec@gmail.com>,
        oleg@redhat.com, pranith kumar <bobby.prani@gmail.com>
Subject: Re: [PATCH RFC tip/core/rcu] SRCU rewrite
Message-ID: <20161117123134.4c0822e0@gmail.com>
In-Reply-To: <CAJhGHyCJtSnL8r6eCrm3b2334pnnLt9yVXnWqr7j=wjA2_RNwQ@mail.gmail.com>
References: <20161114183636.GA28589@linux.vnet.ibm.com>
        <20161115014445.GC12110@tardis.cn.ibm.com>
        <20161115143700.GZ4127@linux.vnet.ibm.com>
        <CAJhGHyCr74zTnc10QvORDZXfd-vdc0Caa0hG250UWwMwsMkUfQ@mail.gmail.com>
        <20161117143012.GB5227@tardis.cn.ibm.com>
        <CAJhGHyCJtSnL8r6eCrm3b2334pnnLt9yVXnWqr7j=wjA2_RNwQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1438
Lines: 39

On Thu, 17 Nov 2016 23:07:02 +0800
Lai Jiangshan <jiangshanlai@gmail.com> wrote:

> On Thu, Nov 17, 2016 at 10:31 PM, Boqun Feng <boqun.feng@gmail.com> wrote:
> > After reading the comment for a while, I actually got a question, maybe
> > I miss something ;-)
> >
> > Why "at most NR_CPUS worth of readers using the old index haven't
> > incremented the counters" could save us from overflow the counter?
> >
> > Please consider the following case in current implementation:
> >
> >
> > {sp->completed = 0} so idx = 1 in srcu_advance_batches(...)
> >
> > one thread A is currently in __srcu_read_lock() and using idx = 1 and
> > about to increase the percpu c[idx], and ULONG_MAX __srcu_read_lock()s
> > have been called and returned with idx = 1, please note I think this is
> > possible because I assume we may have some code like this:
> >
> >         unsigned long i = 0;
> >         for (; i < ULONG_MAX; i++)
> >                 srcu_read_lock(); // return the same idx 1;  
> 
> this is the wrong usage of the api.
> 
> 
> you might rewrite it as:
> 
>         unsigned long index[2] = {0, 0};
>         unsigned long i = 0;
>         for (;  index[1] < ULONG_MAX; i++)
>                  index[srcu_read_lock()]++;
Doesn't this still fail the API by not nesting correctly? If you don't keep
track of the order the indices were returned in then it seems like you would
exit the critical sections in the wrong order.

Thanks,
Lance