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