* [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also [not found] <20260326131838.634095-1-dhowells@redhat.com> @ 2026-03-26 13:18 ` David Howells 2026-03-29 19:12 ` Jakub Kicinski 2026-03-29 19:14 ` Jakub Kicinski 0 siblings, 2 replies; 4+ messages in thread From: David Howells @ 2026-03-26 13:18 UTC (permalink / raw) To: netdev Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni, linux-afs, linux-kernel, Mathieu Desnoyers, John Johansen, Minas Harutyunyan, Simon Horman, apparmor, linux-usb, stable 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, ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also 2026-03-26 13:18 ` [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also David Howells @ 2026-03-29 19:12 ` Jakub Kicinski 2026-03-29 20:01 ` Linus Torvalds 2026-03-29 19:14 ` Jakub Kicinski 1 sibling, 1 reply; 4+ messages in thread From: Jakub Kicinski @ 2026-03-29 19:12 UTC (permalink / raw) To: David Howells Cc: netdev, Marc Dionne, David S. Miller, Eric Dumazet, Paolo Abeni, linux-afs, linux-kernel, Mathieu Desnoyers, John Johansen, Minas Harutyunyan, Simon Horman, apparmor, linux-usb, stable, Linus Torvalds 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also 2026-03-29 19:12 ` Jakub Kicinski @ 2026-03-29 20:01 ` Linus Torvalds 0 siblings, 0 replies; 4+ messages in thread From: Linus Torvalds @ 2026-03-29 20:01 UTC (permalink / raw) To: Jakub Kicinski Cc: David Howells, netdev, Marc Dionne, David S. Miller, Eric Dumazet, Paolo Abeni, linux-afs, linux-kernel, Mathieu Desnoyers, John Johansen, Minas Harutyunyan, Simon Horman, apparmor, linux-usb, stable 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also 2026-03-26 13:18 ` [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also David Howells 2026-03-29 19:12 ` Jakub Kicinski @ 2026-03-29 19:14 ` Jakub Kicinski 1 sibling, 0 replies; 4+ messages in thread From: Jakub Kicinski @ 2026-03-29 19:14 UTC (permalink / raw) To: David Howells Cc: netdev, Marc Dionne, David S. Miller, Eric Dumazet, Paolo Abeni, linux-afs, linux-kernel, Mathieu Desnoyers, John Johansen, Minas Harutyunyan, Simon Horman, apparmor, linux-usb, stable 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-29 20:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260326131838.634095-1-dhowells@redhat.com>
2026-03-26 13:18 ` [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also David Howells
2026-03-29 19:12 ` Jakub Kicinski
2026-03-29 20:01 ` Linus Torvalds
2026-03-29 19:14 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox