linux-kernel - Re: wakeup_pipe_readers/writers() && pipe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whZwWJ4dA-r54eyEZaiVpEK+-9joKid3EyPsHVRGAgEgA@mail.gmail.com>
Date: Mon, 6 Jan 2025 10:23:34 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Manfred Spraul <manfred@...orfullife.com>, Christian Brauner <brauner@...nel.org>, 
	David Howells <dhowells@...hat.com>, WangYuli <wangyuli@...ontech.com>, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: wakeup_pipe_readers/writers() && pipe_poll()

On Mon, 6 Jan 2025 at 08:31, Oleg Nesterov <oleg@...hat.com> 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