2023-04-15 00:44:20

by SeongJae Park

Subject: [PATCH 0/2] mm/slab: trivial fixup for SLAB_TYPESAFE_BY_RCU example code snippet

This patchset is for trivial fixup for SLAB_TYPESAFE_BY_RCU example code
snippet, namely adding missed semicolon and breaking RCU read-side
critical section into smaller ones.

SeongJae Park (2):
mm/slab: add a missing semicolon on SLAB_TYPESAFE_BY_RCU example code
mm/slab: break up RCU readers on SLAB_TYPESAFE_BY_RCU example code

include/linux/slab.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

--
2.25.1

2023-04-15 00:44:22

by SeongJae Park

Subject: [PATCH 1/2] mm/slab: add a missing semicolon on SLAB_TYPESAFE_BY_RCU example code

An example code snippet for SLAB_TYPESAFE_BY_RCU is missing a semicolon.
Add it.

Signed-off-by: SeongJae Park <[email protected]>
---
include/linux/slab.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index f8b1d63c63a3..b18e56c6f06c 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -53,7 +53,7 @@
* stays valid, the trick to using this is relying on an independent
* object validation pass. Something like:
*
- * rcu_read_lock()
+ * rcu_read_lock();
* again:
* obj = lockless_lookup(key);
* if (obj) {
--
2.25.1

2023-04-15 00:45:06

by SeongJae Park

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

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 <[email protected]>
---
include/linux/slab.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

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;
* }
* }
--
2.25.1

2023-04-15 03:05:21

by Matthew Wilcox

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

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;

2023-04-15 03:05:48

by SeongJae Park

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

On Sat, 15 Apr 2023 03:51:54 +0100 Matthew Wilcox <[email protected]> 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

2023-04-15 03:24:23

by SeongJae Park

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

On Sat, 15 Apr 2023 03:04:52 +0000 SeongJae Park <[email protected]> wrote:

> On Sat, 15 Apr 2023 03:51:54 +0100 Matthew Wilcox <[email protected]> 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