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

[Christian Brauner]

On Sun, Feb 02, 2025 at 10:39:16AM -0800, Linus Torvalds wrote:
> 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.

Agreed.