public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: K Prateek Nayak <kprateek.nayak@amd.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Christian Brauner <brauner@kernel.org>,
	Jeff Layton <jlayton@kernel.org>,
	David Howells <dhowells@redhat.com>,
	"Gautham R. Shenoy" <gautham.shenoy@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>,
	"Linus Torvalds" <torvalds@linux-foundation.org>
Subject: Re: [PATCH 1/2] pipe: change pipe_write() to never add a zero-sized buffer
Date: Mon, 10 Feb 2025 05:09:12 +0530	[thread overview]
Message-ID: <b050f92e-4117-4e93-8ec6-ec595fd8570a@amd.com> (raw)
In-Reply-To: <20250209191510.GB27435@redhat.com>

Hello Oleg,

On 2/10/2025 12:45 AM, Oleg Nesterov wrote:
> On 02/09, Linus Torvalds wrote:
>>
>> On Sun, 9 Feb 2025 at 10:45, Oleg Nesterov <oleg@redhat.com> wrote:
>>>
>>> Again, lets look eat_empty_buffer().
>>>
>>> The comment says "maybe it's empty" but how/why can this happen ?
>>
>> WHY DO YOU CARE?
> 
> Because it looks unclear/confusing, and I think it can confuse other
> readers of this code. Especially after 1/2.
> 
>> So here's the deal: either you
> ...
>>   (b) you DON'T convince yourself that that is true, and you leave
>> eat_empty_buffer() alone.
> 
> Yes, I failed to convince myself that fs/splice.c can never add an
> empty bufer. Although it seems to me it should not.
> 
>> In contrast, the "eat_empty_buffer()" case just saying "if it's an
>> empty buffer, it doesn't satisfy my needs, so I'll just release the
>> empty buffer and go on".
> 
> ... without wakeup_pipe_writers().
> 
> OK, nevermind, I see your point even if I do not 100% agree.
> 
> I'll send v2 without WARN_ON() and without 2/2.

Went ahead and tested that version on top of mainline with your
previous change to skip updating {a,c,m}time, here are the results:

==================================================================
Test          : sched-messaging
Units         : Normalized time in seconds
Interpretation: Lower is better
Statistic     : AMean
==================================================================
Case:       mainline + no-acm_time[pct imp](CV)   patched[pct imp](CV)
  1-groups     1.00 [ -0.00]( 7.19)                0.95 [  4.90](12.39)
  2-groups     1.00 [ -0.00]( 3.54)                1.02 [ -1.92]( 6.55)
  4-groups     1.00 [ -0.00]( 2.78)                1.01 [ -0.85]( 2.18)
  8-groups     1.00 [ -0.00]( 1.04)                0.99 [  0.63]( 0.77)
16-groups     1.00 [ -0.00]( 1.02)                1.00 [ -0.26]( 0.98)

I don't see any regression / improvements from a performance standpoint
on my 3rd Generation EPYC system (2 x 64C/128T. boost on, C2 disabled)
Feel free to include:

Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>

> 
> Oleg.
> 

-- 
Thanks and Regards,
Prateek


  reply	other threads:[~2025-02-09 23:39 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
2025-02-09 18:52           ` Linus Torvalds
2025-02-09 19:15             ` Oleg Nesterov
2025-02-09 23:39               ` K Prateek Nayak [this message]
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=b050f92e-4117-4e93-8ec6-ec595fd8570a@amd.com \
    --to=kprateek.nayak@amd.com \
    --cc=Neeraj.Upadhyay@amd.com \
    --cc=brauner@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=gautham.shenoy@amd.com \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=oleg@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox