Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 29 Mar 2023 13:45:54 -0700
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     Frederic Weisbecker <frederic@kernel.org>
Cc:     LKML <linux-kernel@vger.kernel.org>, rcu <rcu@vger.kernel.org>,
        Uladzislau Rezki <urezki@gmail.com>,
        Neeraj Upadhyay <quic_neeraju@quicinc.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        Joel Fernandes <joel@joelfernandes.org>
Subject: Re: [PATCH 1/4] rcu/nocb: Protect lazy shrinker against concurrent
 (de-)offloading
Message-ID: <15ba9151-16ad-4c23-a236-918a061bd893@paulmck-laptop>
Reply-To: paulmck@kernel.org
References: <20230322194456.2331527-1-frederic@kernel.org>
 <20230322194456.2331527-2-frederic@kernel.org>
 <c614c542-f2b5-4b39-bbc4-ae5f0a125c81@paulmck-laptop>
 <ZB4fhA1BafN7h2N3@localhost.localdomain>
 <ae1cb391-aeed-4587-8d9d-50909c918fb1@paulmck-laptop>
 <ZCCknqnceazfyzvr@localhost.localdomain>
 <dd853e13-ffcd-4579-adad-80b014e906ef@paulmck-laptop>
 <ZCRiXrQQRNy2aJAS@lothringen>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZCRiXrQQRNy2aJAS@lothringen>
Precedence: bulk

On Wed, Mar 29, 2023 at 06:07:58PM +0200, Frederic Weisbecker wrote:
> On Sun, Mar 26, 2023 at 02:45:18PM -0700, Paul E. McKenney wrote:
> > On Sun, Mar 26, 2023 at 10:01:34PM +0200, Frederic Weisbecker wrote:
> > > > > > >  	/* Snapshot count of all CPUs */
> > > > > > >  	for_each_possible_cpu(cpu) {
> > > > > > >  		struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> > > > > > > -		int _count = READ_ONCE(rdp->lazy_len);
> > > > > > > +		int _count;
> > > > > > > +
> > > > > > > +		if (!rcu_rdp_is_offloaded(rdp))
> > > > > > > +			continue;
> > > > > > 
> > > > > > If the CPU is offloaded, isn't ->lazy_len guaranteed to be zero?
> > > > > > 
> > > > > > Or can it contain garbage after a de-offloading operation?
> > > > > 
> > > > > If it's deoffloaded, ->lazy_len is indeed (supposed to be) guaranteed to be zero.
> > > > > Bypass is flushed and disabled atomically early on de-offloading and the
> > > > > flush resets ->lazy_len.
> > > > 
> > > > Whew!  At the moment, I don't feel strongly about whether or not
> > > > the following code should (1) read the value, (2) warn on non-zero,
> > > > (3) assume zero without reading, or (4) some other option that is not
> > > > occurring to me.  Your choice!
> > > 
> > > (2) looks like a good idea!
> > 
> > Sounds good to me!
> 
> So since we now iterate rcu_nocb_mask after the patchset, there is no more
> deoffloaded rdp to check. Meanwhile I put a WARN in the new series making
> sure that an rdp in rcu_nocb_mask is also offloaded (heh!)

Sounds good, thank you!

							Thanx, Paul