diff mbox series

[2/2] mm/slab: break up RCU readers on SLAB_TYPESAFE_BY_RCU example code

Message ID 20230415003754.1852-3-sj@kernel.org
State New
Headers
Series mm/slab: trivial fixup for SLAB_TYPESAFE_BY_RCU example code snippet |

Commit Message

SeongJae Park April 15, 2023, 12:37 a.m. UTC 
  The SLAB_TYPESAFE_BY_RCU example code snippet is having not tiny RCU
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.  Apply the change to
SLAB_TYPESAFE_BY_RCU example code snippet, too.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/slab.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Matthew Wilcox April 15, 2023, 2:51 a.m. UTC | #1 
On Sat, Apr 15, 2023 at 12:37:53AM +0000, SeongJae Park wrote:
>   *
> + * begin:
>   *  rcu_read_lock();
> - * again:
>   *  obj = lockless_lookup(key);
>   *  if (obj) {
>   *    if (!try_get_ref(obj)) // might fail for free objects
> - *      goto again;
> + *      goto begin;
>   *
>   *    if (obj->key != key) { // not the object we expected
>   *      put_ref(obj);
> + *      rcu_read_unlock();
>   *      goto again;

- *      goto again;
+ *      goto begin;
SeongJae Park April 15, 2023, 3:04 a.m. UTC | #2 
On Sat, 15 Apr 2023 03:51:54 +0100 Matthew Wilcox <willy@infradead.org> wrote:

> On Sat, Apr 15, 2023 at 12:37:53AM +0000, SeongJae Park wrote:
> >   *
> > + * begin:
> >   *  rcu_read_lock();
> > - * again:
> >   *  obj = lockless_lookup(key);
> >   *  if (obj) {
> >   *    if (!try_get_ref(obj)) // might fail for free objects
> > - *      goto again;
> > + *      goto begin;
> >   *
> >   *    if (obj->key != key) { // not the object we expected
> >   *      put_ref(obj);
> > + *      rcu_read_unlock();
> >   *      goto again;
> 
> - *      goto again;
> + *      goto begin;

Oops, thank you for catching, Matthew!


Thanks,
SJ
SeongJae Park April 15, 2023, 3:23 a.m. UTC | #3 
On Sat, 15 Apr 2023 03:04:52 +0000 SeongJae Park <sj@kernel.org> wrote:

> On Sat, 15 Apr 2023 03:51:54 +0100 Matthew Wilcox <willy@infradead.org> wrote:
> 
> > On Sat, Apr 15, 2023 at 12:37:53AM +0000, SeongJae Park wrote:
> > >   *
> > > + * begin:
> > >   *  rcu_read_lock();
> > > - * again:
> > >   *  obj = lockless_lookup(key);
> > >   *  if (obj) {
> > >   *    if (!try_get_ref(obj)) // might fail for free objects
> > > - *      goto again;
> > > + *      goto begin;

I simply followed the commit da82af04352b ("doc: Update and wordsmith
rculist_nulls.rst") for this change, and I think I overlooked this part.  If
I'm not missing something, I think we should call rcu_read_unlock() here too,
just before 'goto begin'.

I will post another patch for rculist_null.rst later.


Thanks,
SJ

> > >   *
> > >   *    if (obj->key != key) { // not the object we expected
> > >   *      put_ref(obj);
> > > + *      rcu_read_unlock();
> > >   *      goto again;
> > 
> > - *      goto again;
> > + *      goto begin;
> 
> Oops, thank you for catching, Matthew!
> 
> 
> Thanks,
> SJ
diff mbox series

Patch

diff --git a/include/linux/slab.h b/include/linux/slab.h
index b18e56c6f06c..a2fd57322c9c 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -53,15 +53,16 @@ 
  * 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;
+ *      goto begin;
  *
  *    if (obj->key != key) { // not the object we expected
  *      put_ref(obj);
+ *      rcu_read_unlock();
  *      goto again;
  *    }
  *  }