From: Leon Romanovsky <leonro@mellanox.com>
To: Sagi Grimberg <sagi@grimberg.me>
Cc: Max Gurtovoy <maxg@mellanox.com>,
linux-rdma@vger.kernel.org, jgg@nvidia.com, jgg@mellanox.com,
dledford@redhat.com, oren@mellanox.com
Subject: Re: [PATCH 1/2] IB/isert: use unlikely macro in the fast path
Date: Fri, 7 Aug 2020 19:09:56 +0300 [thread overview]
Message-ID: <20200807160956.GO4432@unreal> (raw)
In-Reply-To: <5364b857-fb44-78ab-85e9-d0e6700ae7c1@grimberg.me>
On Thu, Aug 06, 2020 at 12:51:15PM -0700, Sagi Grimberg wrote:
>
> > 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.
>
> I don't see any reason to find evidence in justification here. It's a
> fastpath call, which is unlikely to fail, and these macros are
> considered common practice.
>
> There is no reason to make Max to go and quantify a micro-optimization.
Unfortunately Max didn't try to see if these likely/unlikely macros
change something, but I did.
Simple objdump -d before and after shows that GCC 9 generates same
ISERT code before and after this patch. It is expected and there are a lot
of reasons for that, but all of them can be reduced to two:
* First, GCC is awesome in building profiled code with right predictions for
standard flows.
* Second, likely/unlikely is intended to be used when input/output is random
from GCC point of view.
So as a summary, there is no optimization here, just misuse of unlikely macro.
BTW, old GCCs behave the same and kernel full of wrong copy/paste.
Thanks
next prev parent reply other threads:[~2020-08-07 16:10 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-05 12:12 [PATCH 1/2] IB/isert: use unlikely macro in the fast path Max Gurtovoy
2020-08-05 12:12 ` [PATCH 2/2] IB/isert: remove duplicated error prints Max Gurtovoy
2020-08-06 20:12 ` Sagi Grimberg
2020-08-18 18:41 ` Jason Gunthorpe
2020-08-05 13:16 ` [PATCH 1/2] IB/isert: use unlikely macro in the fast path Leon Romanovsky
2020-08-05 15:14 ` Max Gurtovoy
2020-08-05 16:06 ` Leon Romanovsky
2020-08-05 16:28 ` Max Gurtovoy
2020-08-05 16:37 ` Leon Romanovsky
2020-08-06 10:56 ` Max Gurtovoy
2020-08-06 19:51 ` Sagi Grimberg
2020-08-07 16:09 ` Leon Romanovsky [this message]
2020-08-07 16:33 ` Sagi Grimberg
2020-08-06 19:43 ` Sagi Grimberg
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=20200807160956.GO4432@unreal \
--to=leonro@mellanox.com \
--cc=dledford@redhat.com \
--cc=jgg@mellanox.com \
--cc=jgg@nvidia.com \
--cc=linux-rdma@vger.kernel.org \
--cc=maxg@mellanox.com \
--cc=oren@mellanox.com \
--cc=sagi@grimberg.me \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.