From: Oleg Nesterov <oleg@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christian Brauner <brauner@kernel.org>,
Jeff Layton <jlayton@kernel.org>,
David Howells <dhowells@redhat.com>,
"Gautham R. Shenoy" <gautham.shenoy@amd.com>,
K Prateek Nayak <kprateek.nayak@amd.com>,
Mateusz Guzik <mjguzik@gmail.com>,
Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>,
Oliver Sang <oliver.sang@intel.com>,
Swapnil Sapkal <swapnil.sapkal@amd.com>,
WangYuli <wangyuli@uniontech.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] pipe: change pipe_write() to never add a zero-sized buffer
Date: Sun, 9 Feb 2025 19:44:28 +0100 [thread overview]
Message-ID: <20250209184427.GA27435@redhat.com> (raw)
In-Reply-To: <CAHk-=whirZek1fZQ_gYGHZU71+UKDMa_MYWB5RzhP_owcjAopw@mail.gmail.com>
On 02/09, Linus Torvalds wrote:
>
> On Sun, 9 Feb 2025 at 10:02, Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > Could you explain what do you think should I do if I keep this check?
> > make pipe_buf_assert_len() return void? or just replace it with
> > WARN_ON_ONCE(!buf->len) in its callers?
>
> Just replace it with WARN_ON_ONCE() in any place where you really
> think it's needed.
OK, will do.
> IOW, why warn for a case that isn't a problem, and you're only making
> it a problem by thinking it is?
Again, lets look eat_empty_buffer().
The comment says "maybe it's empty" but how/why can this happen ?
The changelog for d1a819a2ec2d3 ("splice: teach splice pipe reading
about empty pipe buffers") says "you can trigger it by doing a write
to a pipe that fails" but if someone looks at anon_pipe_write() after
1/2 this case is not possible.
And if eat_empty_buffer() flushes the buffer and updates pipe->tail,
why doesn't it wake the writers?
WARN_ON_ONCE() makes it clear that we do not expect !buf->len == 0,
and the kernel will complain if it does happen.
So unless you have a strong opinion, I'd prefer to keep it for now.
Oleg.
next prev parent reply other threads:[~2025-02-09 18:45 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-09 15:07 [PATCH 0/2] pipe: change pipe_write() to never add a zero-sized buffer Oleg Nesterov
2025-02-09 15:07 ` [PATCH 1/2] " Oleg Nesterov
2025-02-09 17:26 ` Linus Torvalds
2025-02-09 18:02 ` Oleg Nesterov
2025-02-09 18:17 ` Linus Torvalds
2025-02-09 18:44 ` Oleg Nesterov [this message]
2025-02-09 18:52 ` Linus Torvalds
2025-02-09 19:15 ` Oleg Nesterov
2025-02-09 23:39 ` K Prateek Nayak
2025-02-10 17:22 ` Oleg Nesterov
2025-02-10 17:37 ` Linus Torvalds
2025-02-10 18:36 ` Oleg Nesterov
2025-02-11 3:59 ` K Prateek Nayak
2025-02-09 15:08 ` [PATCH 2/2] splice: add some pipe_buf_assert_len() checks Oleg Nesterov
2025-02-10 11:40 ` [PATCH v2 1/1] pipe: change pipe_write() to never add a zero-sized buffer Oleg Nesterov
2025-02-10 12:52 ` [PATCH 0/2] " Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250209184427.GA27435@redhat.com \
--to=oleg@redhat.com \
--cc=Neeraj.Upadhyay@amd.com \
--cc=brauner@kernel.org \
--cc=dhowells@redhat.com \
--cc=gautham.shenoy@amd.com \
--cc=jlayton@kernel.org \
--cc=kprateek.nayak@amd.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mjguzik@gmail.com \
--cc=oliver.sang@intel.com \
--cc=swapnil.sapkal@amd.com \
--cc=torvalds@linux-foundation.org \
--cc=wangyuli@uniontech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.