Re: [PATCH 1/2] IB/isert: use unlikely macro in the fast path

[Leon Romanovsky <leonro@mellanox.com>]

On Wed, Aug 05, 2020 at 07:28:50PM +0300, Max Gurtovoy wrote:
>
> On 8/5/2020 7:06 PM, Leon Romanovsky wrote:
> > On Wed, Aug 05, 2020 at 06:14:16PM +0300, Max Gurtovoy wrote:
> > > On 8/5/2020 4:16 PM, Leon Romanovsky wrote:
> > > > On Wed, Aug 05, 2020 at 03:12:30PM +0300, Max Gurtovoy wrote:
> > > > > Add performance optimization that might slightly improve small IO sizes
> > > > > benchmarks.
> > > > >
> > > > > Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> > > > > ---
> > > > >    drivers/infiniband/ulp/isert/ib_isert.c | 4 ++--
> > > > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > > > I find the expectation from "unlikely/likely" keywords to be overrated.
> > > >
> > > > When we introduced dissagregate post send verbs in rdma-core, we
> > > > benchmarked likely/unlikely and didn't find any significant difference
> > > > for code with and without such keywords.
> > > >
> > > > Thanks
> > > Leon,
> > >
> > > We are using these small optimizations in all our ULPs and we saw benefit in
> > > large scale and high loads (we did the same in NVMf/RDMA).
> > >
> > > These kind of optimizations might not be seen immediately but are
> > > accumulated.
> > >
> > > I don't know why do you compare user-space benchmarks to storage drivers.
> > Why not? It produces same asm code and both have same performance
> > characteristic.
> >
> > > Can you please review the code ?
> > There is nothing to review here, the patch is straightforward, I just
> > don't believe in it.
>
> Its ok.
>
> Just ignore it if you don't want to review it.

OK, just because you asked.

I reviewed this patch and didn't find any justification for performance
claim, can you please provide us numbers before/after so we will be able
to decide based on reliable data? It will help us to review our drivers
and improve them even more.

>
> The maintainers of iser target will review and decide if they believe in it
> or not.

Sure, I don't care who will provide numbers.

Thanks

>
>
> > > Sagi,
> > >
> > > Can you send your comments as well ?
> > >
> > >