On Mon, Feb 17, 2025 at 10:22:53AM +0800, Alan Huang wrote: > On Feb 17, 2025, at 10:12, Boqun Feng <boqun.f...@gmail.com> wrote: > > > > Hi Wei, > > > > The change loosk good to me, thanks! > > > > I queued the patch for futher reviews and tests with some changes in the > > commit log (for title formating and a bit more explanation), please see > > below. > > > > Regards, > > Boqun > > > > On Wed, Jan 01, 2025 at 08:23:06AM +0000, Wei Yang wrote: > >> The example code for "Eliminating Stale Data" looks not correct: > >> > >> * rcu_read_unlock() should put after kstrdup() > >> * spin_unlock() should be called before return > >> > >> Signed-off-by: Wei Yang <richard.weiy...@gmail.com> > > [...] > > > > > > ------------------>8 > > Subject: [PATCH] doc/RCU/listRCU: Fix an example code snippets > > > > The example code for "Eliminating Stale Data" looks not correct: > > > > * rcu_read_unlock() should put after kstrdup(), because otherwise > > entry may get freed while kstrdup() is being called. > > > > * spin_unlock() should be called before return, otherwise the > > function would return with the lock of the entry held. > > > > Hence fix these. > > > > Signed-off-by: Wei Yang <richard.weiy...@gmail.com> > > Link: > > https://lore.kernel.org/r/20250101082306.10404-1-richard.weiy...@gmail.com > > Signed-off-by: Boqun Feng <boqun.f...@gmail.com> > > --- > > Documentation/RCU/listRCU.rst | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/RCU/listRCU.rst b/Documentation/RCU/listRCU.rst > > index ed5c9d8c9afe..8df50fcd69fd 100644 > > --- a/Documentation/RCU/listRCU.rst > > +++ b/Documentation/RCU/listRCU.rst > > @@ -348,9 +348,10 @@ to accomplish this would be to add a ``deleted`` flag > > and a ``lock`` spinlock to > > rcu_read_unlock(); > > return AUDIT_BUILD_CONTEXT; > > } > > - rcu_read_unlock(); > > if (state == AUDIT_STATE_RECORD) > > *key = kstrdup(e->rule.filterkey, GFP_ATOMIC); > > + spin_unlock(&e->lock); > > According to the above quick quiz, we should return with the lock held. >
Good catch! I think we then don't need this patch? Wei, are you able to confirm Alan's reasoning here? Regards, Boqun > > + rcu_read_unlock(); > > return state; > > } > > } > > -- > > >
