netdev - Re: [PATCH net-next v3 1/3] rculist: Add __hlist_nulls_replace_rcu() and hlist_nulls_replace_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAVpQUAWijPEtD=1pp-u8tbqWUkJxn94+A12yfdVC0QwAiuTSA@mail.gmail.com>
Date: Tue, 16 Sep 2025 21:43:23 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: luoxuanqiang <xuanqiang.luo@...ux.dev>
Cc: edumazet@...gle.com, kerneljasonxing@...il.com, davem@...emloft.net, 
	kuba@...nel.org, netdev@...r.kernel.org, 
	Xuanqiang Luo <luoxuanqiang@...inos.cn>
Subject: Re: [PATCH net-next v3 1/3] rculist: Add __hlist_nulls_replace_rcu()
 and hlist_nulls_replace_init_rcu()

On Tue, Sep 16, 2025 at 9:27 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
>
> On Tue, Sep 16, 2025 at 8:27 PM luoxuanqiang <xuanqiang.luo@...ux.dev> wrote:
> >
> >
> > 在 2025/9/17 02:58, Kuniyuki Iwashima 写道:
> > > On Tue, Sep 16, 2025 at 3:31 AM <xuanqiang.luo@...ux.dev> wrote:
> > >> From: Xuanqiang Luo <luoxuanqiang@...inos.cn>
> > >>
> > >> Add two functions to atomically replace RCU-protected hlist_nulls entries.
> > >>
> > >> Signed-off-by: Xuanqiang Luo <luoxuanqiang@...inos.cn>
> > >> ---
> > >>   include/linux/rculist_nulls.h | 61 +++++++++++++++++++++++++++++++++++
> > >>   1 file changed, 61 insertions(+)
> > >>
> > >> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
> > >> index 89186c499dd4..8ed604f65a3e 100644
> > >> --- a/include/linux/rculist_nulls.h
> > >> +++ b/include/linux/rculist_nulls.h
> > >> @@ -152,6 +152,67 @@ static inline void hlist_nulls_add_fake(struct hlist_nulls_node *n)
> > >>          n->next = (struct hlist_nulls_node *)NULLS_MARKER(NULL);
> > >>   }
> > >>
> > >> +/**
> > >> + * __hlist_nulls_replace_rcu - replace an old entry by a new one
> > >> + * @old: the element to be replaced
> > >> + * @new: the new element to insert
> > >> + *
> > >> + * Description:
> > >> + * Replace the old entry with the new one in a RCU-protected hlist_nulls, while
> > >> + * permitting racing traversals.
> > >> + *
> > >> + * The caller must take whatever precautions are necessary (such as holding
> > >> + * appropriate locks) to avoid racing with another list-mutation primitive, such
> > >> + * as hlist_nulls_add_head_rcu() or hlist_nulls_del_rcu(), running on this same
> > >> + * list.  However, it is perfectly legal to run concurrently with the _rcu
> > >> + * list-traversal primitives, such as hlist_nulls_for_each_entry_rcu().
> > >> + */
> > >> +static inline void __hlist_nulls_replace_rcu(struct hlist_nulls_node *old,
> > >> +                                            struct hlist_nulls_node *new)
> > >> +{
> > >> +       struct hlist_nulls_node *next = old->next;
> > >> +
> > >> +       new->next = next;
> >
> > Do we need to use WRITE_ONCE() here, as mentioned in efd04f8a8b45
> > ("rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls")?
> > I am more inclined to think that it is necessary.
>
> Good point, then WRITE_ONCE() makes sense.

and it would be nice to have such reasoning in the commit
message as we were able to learn from efd04f8a8b45.