Re: [PATCH v3] pipe: use __pipe_{lock,unlock} instead of spinlock

From: Sedat Dilek <sedat.dilek@gmail.com>
To: Hongchen Zhang <zhanghongchen@loongson.cn>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Howells <dhowells@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	"Christian Brauner (Microsoft)" <brauner@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	"Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
	Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] pipe: use __pipe_{lock,unlock} instead of spinlock
Date: Mon, 16 Jan 2023 02:52:32 +0100	[thread overview]
Message-ID: <CA+icZUXWAu_+KT9wYfdn7uSp1=ikO5ZdhM2VFokRi_JfhL455Q@mail.gmail.com> (raw)
In-Reply-To: <CA+icZUU3-t0+NhdMQ39OeuwR13eMVOKVhLwS31WTHQ1ksaWgNg@mail.gmail.com>

On Fri, Jan 13, 2023 at 10:32 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Fri, Jan 13, 2023 at 4:19 AM Hongchen Zhang
> <zhanghongchen@loongson.cn> wrote:
> >
> > Hi All,
> > any question about this patch, can it be merged?
> >
> > Thanks
> > On 2023/1/7 am 9:23, Hongchen Zhang wrote:
> > > Use spinlock in pipe_read/write cost too much time,IMO
> > > pipe->{head,tail} can be protected by __pipe_{lock,unlock}.
> > > On the other hand, we can use __pipe_{lock,unlock} to protect
> > > the pipe->{head,tail} in pipe_resize_ring and
> > > post_one_notification.
> > >
> > > Reminded by Matthew, I tested this patch using UnixBench's pipe
> > > test case on a x86_64 machine,and get the following data:
> > > 1) before this patch
> > > System Benchmarks Partial Index  BASELINE       RESULT    INDEX
> > > Pipe Throughput                   12440.0     493023.3    396.3
> > >                                                          ========
> > > System Benchmarks Index Score (Partial Only)              396.3
> > >
> > > 2) after this patch
> > > System Benchmarks Partial Index  BASELINE       RESULT    INDEX
> > > Pipe Throughput                   12440.0     507551.4    408.0
> > >                                                          ========
> > > System Benchmarks Index Score (Partial Only)              408.0
> > >
> > > so we get ~3% speedup.
> > >
> > > Reminded by Andrew, I tested this patch with the test code in
> > > Linus's 0ddad21d3e99 add get following result:
>
> Happy new 2023 Hongchen Zhang,
>
> Thanks for the update and sorry for the late response.
>
> Should be "...s/add/and get following result:"
>
> I cannot say much about the patch itself or tested it in my build-environment.
>
> Best regards,
> -Sedat-
>

I have applied v3 on top of Linux v6.2-rc4.

Used pipebench for a quick testing.

# fdisk -l /dev/sdb
Disk /dev/sdb: 14,91 GiB, 16013942784 bytes, 31277232 sectors
Disk model: SanDisk iSSD P4
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x74f02dea

Device     Boot Start      End  Sectors  Size Id Type
/dev/sdb1        2048 31277231 31275184 14,9G 83 Linux

# cat /dev/sdb | pipebench > /dev/null
Summary:
Piped   14.91 GB in 00h01m34.20s:  162.12 MB/second

Not tested/benchmarked with the kernel w/o your patch.

-Sedat-