Re: [PATCH] pipe_read: don't wake up the writer if the pipe is still full

[Linus Torvalds]

On Sun, 2 Feb 2025 at 09:02, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> And if we do care about performance... Could you look at the trivial patch
> at the end? I don't think {a,c,m}time make any sense in the !fifo case, but
> as you explained before they are visible to fstat() so we probably shouldn't
> remove file_accessed/file_update_time unconditionally.

I dislike that patch because if we actually want to do this, I don't
think you are going far enough.

Yeah, you may stop updating the time, but you still do that
sb_start_write_trylock(), and you still call out to
file_update_time(), and it's all fairly expensive.

So the short-circuiting happens too late, and it happens using a flag
that is non-standard and only with a system call that almost nobody
actually uses (ie 'pipe2()' rather than the normal 'pipe()').

Put another way: if we really care about this, we should just be a lot
more aggressive.

Yes, the time is visible in fstat(). Yes, we've done this forever. But
if the time update is such a big thing, let's go all in, and just see
if anybody really notices?

For example, for tty's, a few years ago we intentionally started doing
time updates only every few seconds, because it was leaking keyboard
timing information (see tty_update_time()). Nobody ever complained.

So I'd actually favor a "let's just remove time updates entirely for
unnamed pipes", and see if anybody notices. Simpler and more
straightforward.

And yes, maybe somebody *does* notice, and we'll have to revisit.

IOW, if you care about this, I'd *much* rather try to do the big and
simple approach _first_. Not do something small and timid that nobody
will actually ever use and that complicates the code.

                Linus