Re: wakeup_pipe_readers/writers() && pipe_poll()

From: Linus Torvalds
Date: Mon Jan 06 2025 - 13:24:05 EST


On Mon, 6 Jan 2025 at 08:31, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> To be honest, I don't understand the wait_address check in poll_wait(),
> it seems that wait_address is never NULL.

Oh, it seems to be historical.

That *used* to be how select worked - once select() or poll() had seen
that somebody returns a "I have data", they set wait_address to NULL
because there's no point in adding any wait-queues any more at that
point.

But these days they do that

wait->_qproc = NULL;

thing instead.

It seems to go back to 626cf2366085 ("poll: add
poll_requested_events() and poll_does_not_wait() functions").

So yeah, I guess these days the wait_table pointer is never NULL
(where "these days" is the last decade+).

> That is what I tried to propose. Will you agree with this change?
> We can even use smp_store_mb(), say

I think it's clearer to just use smp_mb().

The whole smp_store_mb() thing is a pretty random thing, I think we
could / should probably just remove it. It's basically a combination
of "atomic store" and "smp_mb()", and at one point we thought that
doing it with an "xchg" instruction would be better on x86.

And I don't think the one or two byte shorter instruction sequence is
worth it, definitely not for something like updating
entry->wait_address where there's no actual point to making the store
itsdelf atomic.

Linus