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;
From:   SeongJae Park <sj@kernel.org>
To:     Vlastimil Babka <vbabka@suse.cz>
Cc:     SeongJae Park <sj@kernel.org>, akpm@linux-foundation.org,
        willy@infradead.org, paulmck@kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] mm/slab: break up RCU readers on SLAB_TYPESAFE_BY_RCU example code
Date:   Mon, 17 Apr 2023 17:26:57 +0000
Message-Id: <20230417172657.21925-1-sj@kernel.org>
In-Reply-To: <e2561303-8853-7e16-7eba-001415d34e09@suse.cz>
References: 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Vlastimil,

On Mon, 17 Apr 2023 13:05:40 +0200 Vlastimil Babka <vbabka@suse.cz> wrote:

> On 4/15/23 05:31, SeongJae Park wrote:
> > The SLAB_TYPESAFE_BY_RCU example code snippet is having not tiny RCU
> 
> Since "tiny RCU" means something quite specific in the RCU world, it can be
> confusing to read it in this sense. We could say e.g. "... snippet uses a
> single RCU read-side critical section for retries"?

Looks much better, thank you for this suggestion!

> 
> > read-side critical section.  'Documentation/RCU/rculist_nulls.rst' has
> > similar example code snippet, and commit da82af04352b ("doc: Update and
> > wordsmith rculist_nulls.rst") has broken it.
> 
> "has broken it" has quite different meaning than "has broken it up" :) I
> guess we could just add the "up", unless someone has an even better wording.

Good point, thank you for your suggestion!

I will apply above suggestion on the next spin.


Thanks,
SJ

> 
> > Apply the change to
> > SLAB_TYPESAFE_BY_RCU example code snippet, too.
> > 
> > Signed-off-by: SeongJae Park <sj@kernel.org>
> > ---
> >  include/linux/slab.h | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index b18e56c6f06c..6acf1b7c6551 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -53,16 +53,18 @@
> >   * stays valid, the trick to using this is relying on an independent
> >   * object validation pass. Something like:
> >   *
> > + * begin:
> >   *  rcu_read_lock();
> > - * again:
> >   *  obj = lockless_lookup(key);
> >   *  if (obj) {
> >   *    if (!try_get_ref(obj)) // might fail for free objects
> > - *      goto again;
> > + *      rcu_read_unlock();
> > + *      goto begin;
> >   *
> >   *    if (obj->key != key) { // not the object we expected
> >   *      put_ref(obj);
> > - *      goto again;
> > + *      rcu_read_unlock();
> > + *      goto begin;
> >   *    }
> >   *  }
> >   *  rcu_read_unlock();
>