[PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[David Howells]

Unfortunately, list_empty() is not usable with an entry that has been
removed from a list with list_del_rcu() as ->next must be left pointing at
the following entry so as not to break traversal under RCU.

Solve this by moving on_list_rcu() from AppArmor to linux/list.h, and
turning it into an inline function.

Also add an on_list() counterpart (functionally, this is just an antonym
for list_empty()), but the name looks less awkward when applied to a
non-head element.  We probably don't want to use on_list_rcu() generally
because it requires an extra check as ->prev is set differently in the two
cases.

Further, rename the on_list() function in the Designware usb2 drd ip driver
to dwc2_on_list() to free up the original name.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
cc: John Johansen <john.johansen@canonical.com>
cc: Minas Harutyunyan <hminas@synopsys.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: apparmor@lists.ubuntu.com
cc: linux-usb@vger.kernel.org
cc: netdev@vger.kernel.org
cc: stable@kernel.org
---
 drivers/usb/dwc2/gadget.c          |  6 +++---
 include/linux/list.h               | 26 ++++++++++++++++++++++++++
 security/apparmor/include/policy.h |  2 --
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index d216e26c787b..04b6aef8ac13 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4306,11 +4306,11 @@ static int dwc2_hsotg_ep_disable_lock(struct usb_ep *ep)
 }
 
 /**
- * on_list - check request is on the given endpoint
+ * dwc2_on_list - check request is on the given endpoint
  * @ep: The endpoint to check.
  * @test: The request to test if it is on the endpoint.
  */
-static bool on_list(struct dwc2_hsotg_ep *ep, struct dwc2_hsotg_req *test)
+static bool dwc2_on_list(struct dwc2_hsotg_ep *ep, struct dwc2_hsotg_req *test)
 {
 	struct dwc2_hsotg_req *req, *treq;
 
@@ -4338,7 +4338,7 @@ static int dwc2_hsotg_ep_dequeue(struct usb_ep *ep, struct usb_request *req)
 
 	spin_lock_irqsave(&hs->lock, flags);
 
-	if (!on_list(hs_ep, hs_req)) {
+	if (!dwc2_on_list(hs_ep, hs_req)) {
 		spin_unlock_irqrestore(&hs->lock, flags);
 		return -EINVAL;
 	}
diff --git a/include/linux/list.h b/include/linux/list.h
index 00ea8e5fb88b..d224e7210d1b 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -381,6 +381,32 @@ static inline int list_empty(const struct list_head *head)
 	return READ_ONCE(head->next) == head;
 }
 
+/**
+ * on_list - Test whether an entry is on a list.
+ * @entry: The entry to check
+ *
+ * Test whether an entry is on a list.  Safe to use on an entry initialised
+ * with INIT_LIST_HEAD() or LIST_HEAD() or removed with things like
+ * list_del_init().  Not safe for use with list_del() or list_del_rcu().
+ */
+static inline bool on_list(const struct list_head *entry)
+{
+	return !list_empty(entry);
+}
+
+/**
+ * on_list_rcu - Test whether an entry is on a list (RCU-del safe).
+ * @entry: The entry to check
+ *
+ * Test whether an entry is on a list.  Safe to use on an entry initialised
+ * with INIT_LIST_HEAD() or LIST_HEAD() or removed with things like
+ * list_del_init().  Also safe for use with list_del() or list_del_rcu().
+ */
+static inline bool on_list_rcu(const struct list_head *entry)
+{
+	return !list_empty(entry) && entry->prev != LIST_POISON2;
+}
+
 /**
  * list_del_init_careful - deletes entry from list and reinitialize it.
  * @entry: the element to delete from the list.
diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index 3895f8774a3f..c3697c23bbed 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -57,8 +57,6 @@ extern const char *const aa_profile_mode_names[];
 
 #define profile_is_stale(_profile) (label_is_stale(&(_profile)->label))
 
-#define on_list_rcu(X) (!list_empty(X) && (X)->prev != LIST_POISON2)
-
 /* flags in the dfa accept2 table */
 enum dfa_accept_flags {
 	ACCEPT_FLAG_OWNER = 1,

Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[Jakub Kicinski]

On Thu, 26 Mar 2026 13:18:29 +0000 David Howells wrote:
> diff --git a/include/linux/list.h b/include/linux/list.h
> index 00ea8e5fb88b..d224e7210d1b 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -381,6 +381,32 @@ static inline int list_empty(const struct list_head *head)
>  	return READ_ONCE(head->next) == head;
>  }
>  
> +/**
> + * on_list - Test whether an entry is on a list.
> + * @entry: The entry to check
> + *
> + * Test whether an entry is on a list.  Safe to use on an entry initialised
> + * with INIT_LIST_HEAD() or LIST_HEAD() or removed with things like
> + * list_del_init().  Not safe for use with list_del() or list_del_rcu().
> + */
> +static inline bool on_list(const struct list_head *entry)
> +{
> +	return !list_empty(entry);
> +}
> +
> +/**
> + * on_list_rcu - Test whether an entry is on a list (RCU-del safe).
> + * @entry: The entry to check
> + *
> + * Test whether an entry is on a list.  Safe to use on an entry initialised
> + * with INIT_LIST_HEAD() or LIST_HEAD() or removed with things like
> + * list_del_init().  Also safe for use with list_del() or list_del_rcu().
> + */
> +static inline bool on_list_rcu(const struct list_head *entry)
> +{
> +	return !list_empty(entry) && entry->prev != LIST_POISON2;
> +}

Could someone with sufficient weight to their name ack this?

The non-RCU version of on_list() does not sit well with me.
It provides no additional semantics above list_empty() and
the uninit / poison-related gotchas more obvious when typing
!list_empty(&entry->list).

I can believe the RCU version is more useful. It could probably
be used on both RCU and non-RCU entries?

Last minor nit - the list API consistently uses list_ as a prefix.
I have no better name to suggest but it's sad that on_list_rcu()
breaks that.

I think you're missing a READ_ONCE(), BTW.

Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[Linus Torvalds]

On Sun, 29 Mar 2026 at 12:12, Jakub Kicinski <kuba@kernel.org> wrote:
>
> Could someone with sufficient weight to their name ack this?

I don't particularly like it. I think the name is too generic, and
it's all wrong anyway. Whether something is on a list or not ends up
being much too specific to the use-case, and I do *not* see a huge
advantage to a helper function that just wraps "list_empty()" with
another name that is actually *less* descriptive.

So no. NAK.

As you mention, the RCU version at least does something else, but
honestly, it looks pretty damn questionable too. And no, it does not
work for non-RCU lists in any kind of generic sense, since iut's
perfectly valid to remove list entries without poisoning them.,

For example, some places that want a simple "am I on a list" will use
__list_del_clearprev(), which does *not* poison the prev pointer, but
just clears it instead. And then the "am I on a list" is just checking
prev for NULL or not.

In other words: all of this is wrong. Whether you are on a list or not
is simply not a generic operation. It depends on the user.

The name is also *MUCH* too generic anyway.

               Linus

Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[David Howells]

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> ... and I do *not* see a huge advantage to a helper function that just wraps
> "list_empty()" with another name that is actually *less* descriptive.

I don't like list_empty() as the name of the function used to find out whether
an entry is on a list.  Yes, technically, all it's doing is seeing if the
list_head is 'empty', but, linguistically, it looks wrong: the question you're
asking is not if the list is empty (you're not looking at the list head), but
if the entry is on a list.

So if I see in the code:

	if (list_empty(p))

what is the test actually asking?

Note that various other list types in the kernel have separate "is the list
empty" and "is the entry on a list" primitives, though, granted, usually
because they require separate functions programmatically.

Anyway, I'll find a different way to do this, not involving checking the prev
pointer.  What I don't want to do is hard code "prev == LIST_POISON2" into my
stuff.  Anything like that really needs to be in list.h.

David

Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[Linus Torvalds]

On Mon, 30 Mar 2026 at 03:49, David Howells <dhowells@redhat.com> wrote:
>
> Anyway, I'll find a different way to do this, not involving checking the prev
> pointer.  What I don't want to do is hard code "prev == LIST_POISON2" into my
> stuff.  Anything like that really needs to be in list.h.

So i think the proper model is:

(a) normal and good list users should never *use* this kind of "is
this entry on a list or not".

Dammit, you should *KNOW* that already from core logic. Not with a
flag, not with a function to ask, but from how things work. The whole
"am I on a list or not" should not be a list issue, it should be
obvious.

(b) if the code in question really doesn't know what the ^%&%^ it did,
and has lost sight of what it has done to a list entry, and really
wants some kind of "did I remove this entry already" logic, I would
encourage such uses to either re-consider, or just use the
"__list_del_clearprev()" function when removing entries.

Because I really don't want the core list handling to cater to code
that doesn't know what the hell it has done.

                Linus

Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[David Howells]

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Dammit, you should *KNOW* that already from core logic. Not with a
> flag, not with a function to ask, but from how things work. The whole
> "am I on a list or not" should not be a list issue, it should be
> obvious.

The circumstance in question is this: There's a list of outstanding calls
attached to the rxrpc network namespace.  Calls may hang around on it beyond
the life of the socket that created them for a little bit to deal with network
protocol cleanup, timer cleanup, work func cleanup.  Under normal operation,
calls are removed as the last ref is put.

However, should the namespace be deleted, rxrpc_destroy_all_calls() trawls the
list to report any calls that haven't been cleaned up and the calls are
deleted from the list as it reports them so that the spinlock doesn't have to
be kept held.  It used to do other work here too, IIRC, but that's no longer
the case, so perhaps this loop is not needed now, and a warning will suffice
if the list is not empty (or I could just report, say, the first 10 calls and
not worry about calling cond_resched()).  The wait at the bottom of the
function should be sufficient to hold up namespace deallocation.

If I don't delete entries in rxrpc_destroy_all_calls(), then rxrpc_put_call()
only needs list_empty() to guard against the call not having being queued yet.
I could have a flag for that, but it would be superfluous.

Note that the reason for the RCU walking is because /proc/net/rxrpc/calls
walks over the same list, so I still need the next patch which switches to
list_del_rcu().

David

Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[Linus Torvalds]

On Mon, 30 Mar 2026 at 16:50, David Howells <dhowells@redhat.com> wrote:
>
> If I don't delete entries in rxrpc_destroy_all_calls(), then rxrpc_put_call()
> only needs list_empty() to guard against the call not having being queued yet.
> I could have a flag for that, but it would be superfluous.

So make *that* code use a creaful "delete with flag".

As far as I know, __list_del_clearprev() works fine for RCU walking
too, and that "prev is NULL" works as a "this is not on a list".

Admittedly I didn't think about it a lot.

So my point is more that this should not be some "generic list"
behavior, and I do *not* want people to think that they can just do
"is_on_list()" kind of crap in general.

This should be a "this user needs that particular behavior, and has
used this particular function to get it".

And yes, this pattern started out as a single performance-critical
networking user, and maybe we could rename and codify this pattern
better since we now have a couple of users (bpf and xdp) and another
apparently appearing. But I think that "rename and codify" should be a
separate thing (and done after ths particular issue is fixed).

              Linus

Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[Jakub Kicinski]

On Mon, 30 Mar 2026 15:14:07 -0700 Linus Torvalds wrote:
> On Mon, 30 Mar 2026 at 03:49, David Howells <dhowells@redhat.com> wrote:
> >
> > Anyway, I'll find a different way to do this, not involving checking the prev
> > pointer.  What I don't want to do is hard code "prev == LIST_POISON2" into my
> > stuff.  Anything like that really needs to be in list.h.  
> 
> So i think the proper model is:
> 
> (a) normal and good list users should never *use* this kind of "is
> this entry on a list or not".
> 
> Dammit, you should *KNOW* that already from core logic. Not with a
> flag, not with a function to ask, but from how things work. The whole
> "am I on a list or not" should not be a list issue, it should be
> obvious.

+1 FWIW, the use of the on_list_rcu() in patch 5 looks kinda shady:

@@ -654,9 +654,9 @@ void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace why)
 	if (dead) {
 		ASSERTCMP(__rxrpc_call_state(call), ==, RXRPC_CALL_COMPLETE);
 
-		if (!list_empty(&call->link)) {
+		if (on_list_rcu(&call->link)) {
 			spin_lock(&rxnet->call_lock);
-			list_del_init(&call->link);
+			list_del_rcu(&call->link);
 			spin_unlock(&rxnet->call_lock);
 		}

I haven't dug around to see if there's some higher level lock
protecting the whole op, so I didn't mention it. But I was worried
that on_list() would lead to questionable code, and the first use
didn't deliver the reassurance I was hoping for.

> (b) if the code in question really doesn't know what the ^%&%^ it did,
> and has lost sight of what it has done to a list entry, and really
> wants some kind of "did I remove this entry already" logic, I would
> encourage such uses to either re-consider, or just use the
> "__list_del_clearprev()" function when removing entries.
> 
> Because I really don't want the core list handling to cater to code
> that doesn't know what the hell it has done.

Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also

[Jakub Kicinski]

On Thu, 26 Mar 2026 13:18:29 +0000 David Howells wrote:
> Unfortunately, list_empty() is not usable with an entry that has been
> removed from a list with list_del_rcu() as ->next must be left pointing at
> the following entry so as not to break traversal under RCU.

This seems to break build for jffs.
Someone already marked this as rejected in PW.
Whoever it was PLEASE STOP.

Quoting documentation:

  Updating patch status
  ~~~~~~~~~~~~~~~~~~~~~
  
  Contributors and reviewers do not have the permissions to update patch
  state directly in patchwork. Patchwork doesn't expose much information
  about the history of the state of patches, therefore having multiple
  people update the state leads to confusion.
  
  Instead of delegating patchwork permissions netdev uses a simple mail
  bot which looks for special commands/lines within the emails sent to
  the mailing list. For example to mark a series as Changes Requested
  one needs to send the following line anywhere in the email thread::
  
    pw-bot: changes-requested
  
  As a result the bot will set the entire series to Changes Requested.
  This may be useful when author discovers a bug in their own series
  and wants to prevent it from getting applied.
  
  The use of the bot is entirely optional, if in doubt ignore its
  existence completely. Maintainers will classify and update the state
  of the patches themselves. No email should ever be sent to the list
  with the main purpose of communicating with the bot, the bot commands
  should be seen as metadata. 
  The use of the bot is restricted to authors of the patches (the
  ``From:`` header on patch submission and command must match!),
  maintainers of the modified code according to the MAINTAINERS file
  (again, ``From:`` must match the MAINTAINERS entry) and a handful of
  senior reviewers. 
  Bot records its activity here:
  
    https://netdev.bots.linux.dev/pw-bot.html
  
See:
  https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#updating-patch-status