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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 8/5/2020 7:37 PM, Leon Romanovsky wrote:
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@xxxxxxxxxxxx>
---
    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.

As I said, these are incremental optimizations that probably won't be seen immediately with 1 or 2 changes. But accumulated small optimizations can reach to 3%-4%.

If you don't believe in this patch - ignore it and review others. I'm sure you have a lot. Let other maintainers review it.

You're also welcomed to remove the likely/unlikely macros from all Linux kernel and let's see what comments will it get from other maintainers.

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.

I'm not talking about providing numbers.


Thanks


Sagi,

Can you send your comments as well ?





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux