Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also
From: Jakub Kicinski
Date: Mon Mar 30 2026 - 20:12:41 EST
On Mon, 30 Mar 2026 15:14:07 -0700 Linus Torvalds wrote:
> On Mon, 30 Mar 2026 at 03:49, David Howells <dhowells@xxxxxxxxxx> 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.