[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20201001155641.3421-1-mailhol.vincent@wanadoo.fr>
Date: Fri, 2 Oct 2020 00:56:41 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Vincent Mailhol <mailhol.vincent@...adoo.fr>,
Jakub Kicinski <kuba@...nel.org>,
Oliver Neukum <oneukum@...e.com>,
Masahiro Yamada <masahiroy@...nel.org>,
Arunachalam Santhanam <arunachalam.santhanam@...bosch.com>,
Marc Kleine-Budde <mkl@...gutronix.de>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
linux-can@...r.kernel.org, Wolfgang Grandegger <wg@...ndegger.com>,
"David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2 5/6] can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces
> > + num_element =
> > + es58x_msg_num_element(es58x_dev->dev,
> > + bulk_rx_loopback_msg->rx_loopback_msg,
> > + msg_len);
> > + if (unlikely(num_element <= 0))
> > + return num_element;
>
> Meta-comment on your use of 'unlikely' everywhere. Please drop it, it's
> only to be used if you can actually measure the difference in a
> benchmark. You are dealing with USB devices, which are really really
> slow here. Also, humans make horrible guessers for this type of thing,
> the compiler and CPU can get this right much more often than we can, and
> we had the numbers for it (someone measured that 80-90% of our usages of
> these markings are actually wrong on modern cpus).
>
> So just drop them all, it makes the code simpler to read and understand,
> and the cpu can actually go faster.
All those branch on which the unlikely() macro were applied were
supposed to never been executed under normal circumstances. But I
indeed have no benchmark to claim such use.
Thank you for the detailed explanation, makes perfect sense. Each use
of the likely()/unlikely() macros will be removed in v3 revision.
Powered by blists - more mailing lists