Re: [PATCH for-next 4/4] block/rnbd: Remove all likely and unlikely

From: Gioh Kim <gi-oh.kim@ionos.com>
To: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"hch@infradead.org" <hch@infradead.org>,
	"sagi@grimberg.me" <sagi@grimberg.me>,
	"bvanassche@acm.org" <bvanassche@acm.org>,
	"haris.iqbal@ionos.com" <haris.iqbal@ionos.com>,
	"jinpu.wang@ionos.com" <jinpu.wang@ionos.com>
Subject: Re: [PATCH for-next 4/4] block/rnbd: Remove all likely and unlikely
Date: Thu, 29 Apr 2021 09:14:04 +0200	[thread overview]
Message-ID: <CAJX1YtbJXWL7du8+6QzMisQhF72zRfMx92G2_LiLd3Nj6JSUYg@mail.gmail.com> (raw)
In-Reply-To: <BYAPR04MB496504E1348ABFEE30FFC0C186409@BYAPR04MB4965.namprd04.prod.outlook.com>

On Wed, Apr 28, 2021 at 8:33 PM Chaitanya Kulkarni
<Chaitanya.Kulkarni@wdc.com> wrote:
>
> On 4/27/21 23:14, Gioh Kim wrote:
> > The IO performance test with fio after removing the likely and
> > unlikely macros in all if-statement shows no performance drop.
> > They do not help for the performance of rnbd.
> >
> > The fio test did random read on 32 rnbd devices and 64 processes.
> > Test environment:
> > - AMD Opteron(tm) Processor 6386 SE
> > - 125G memory
> > - kernel version: 5.4.86
>
> why 5.4 and not linux-block/for-next ?

We have done porting only 5.4 for the server machine yet.

>
> > - gcc version: gcc (Debian 8.3.0-6) 8.3.0
> > - Infiniband controller: InfiniBand: Mellanox Technologies MT26428
> > [ConnectX VPI PCIe 2.0 5GT/s - IB QDR / 10GigE] (rev b0)
> >
> > before
> > read: IOPS=549k, BW=2146MiB/s
> > read: IOPS=544k, BW=2125MiB/s
> > read: IOPS=553k, BW=2158MiB/s
> > read: IOPS=535k, BW=2089MiB/s
> > read: IOPS=543k, BW=2122MiB/s
> > read: IOPS=552k, BW=2154MiB/s
> > average: IOPS=546k, BW=2132MiB/s
> >
> > after
> > read: IOPS=556k, BW=2172MiB/s
> > read: IOPS=561k, BW=2191MiB/s
> > read: IOPS=552k, BW=2156MiB/s
> > read: IOPS=551k, BW=2154MiB/s
> > read: IOPS=562k, BW=2194MiB/s
> > -----------
> > average: IOPS=556k, BW=2173MiB/s
> >
> > The IOPS and bandwidth got better slightly after removing
> > likely/unlikely. (IOPS= +1.8% BW= +1.9%) But we cannot make sure
> > that removing the likely/unlikely help the performance because it
> > depends on various situations. We only make sure that removing the
> > likely/unlikely does not drop the performance.
>
> Did you get a chance to collect perf numbers to see which functions are
> getting faster ?

I knew somebody would ask for it ;-)
No, I didn't because I have been occupied with another task.
But I will check it soon in a few weeks.

Thank you for the review.

>
>