Re: [RFC 1/1] rwsem: Shrink rwsem by one pointer

[Linus Torvalds]

On Tue, 17 Feb 2026 at 11:08, Matthew Wilcox (Oracle)
<willy@xxxxxxxxxxxxx> wrote:
>
> Instead of embedding a list_head in struct rw_semaphore, store a pointer
> to the first waiter.  The list of waiters remains a doubly linked list
> so we can efficiently add to the tail of the list, remove from the front
> (or middle) of the list.
>
> Some of the list manipulation becomes more complicated, but it's a
> reasonable tradeoff on the slow paths to shrink some core data structures
> like struct inode.

I like this, but I have to say that I dislike how rwsem_add_waiter()
in particular ends up looking.

Not because it's horrible on its own, but when you look at the
call-sites, that function ends up being entirely pointless.

It does two things:

 - it asserts that the wait_lock is held

 - it now checks whether the new waiter is the first one and does two
different things depending on that

And lookie here: there are exactly two call sites, and both of them
are immediately preceded by a

        raw_spin_lock_irq(&sem->wait_lock);

and one of them then goes on to check whether there are no waiters
before it calls rwsem_add_waiter().

And the other? Immediately *after* the call, it does

        /* we're now waiting on the lock */
        if (rwsem_first_waiter(sem) != &waiter) {

so that other call-site *ALSO* basically ends up having special code
for "am I the first writer"?

Put another way: this helper function seems all kinds of pointless and
it got worse in this iteration, because the pointlessness is now in
some of that added complexity.

So considering that there iareonly two callers. and both of them
*already* fundamentally know about this whole "first waiter is
special" situation, that helper is actually the opposite of a helper.
It's actually hiding what is going on, and just making code generation
worse.

If we want helper functions, can we please just make them be
"add_first_waiter()" and "add_to_waiter_list()", and make them
actually be what the callers need and want?

Somewhat similarly, I also reacted to this part:

    -#define rwsem_first_waiter(sem) \
    -       list_first_entry(&sem->wait_list, struct rwsem_waiter, list)
    +#define rwsem_first_waiter(sem)        sem->first_waiter

that rwsem_first_waiter() macro used to make sense as a syntactic
helper function. But now it really doesn't. It is literally more
typing and *less* legible than just accessing that new
"sem->first_waiter" field.

In other words, I like the direction you're taking, and I think the
code already is set up to have that whole "first waiter" thing that
your patch just makes much more explicit and obvious. That all argues
that yes, having a "first waiter" pointer instead of a "struct
list_head" is a good change.

But I think some of those existing helpers were because we did *not*
use to do it that better way, and they were designed for the old world
order, and they no longer make any sense with your changes.

Hmm?

               Linus