netdev - Re: [PATCH net-next 3/5] drivers/net/phy: add connection between ethtool and phylib for PLCA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y7atfqnih0bQArjf@unreal>
Date:   Thu, 5 Jan 2023 12:59:10 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Piergiorgio Beruto <piergiorgio.beruto@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, Oleksij Rempel <o.rempel@...gutronix.de>
Subject: Re: [PATCH net-next 3/5] drivers/net/phy: add connection between
 ethtool and phylib for PLCA

On Thu, Jan 05, 2023 at 11:49:18AM +0100, Piergiorgio Beruto wrote:
> On Thu, Jan 05, 2023 at 11:03:46AM +0200, Leon Romanovsky wrote:
> > On Wed, Jan 04, 2023 at 03:06:30PM +0100, Piergiorgio Beruto wrote:
> > > This patch adds the required connection between netlink ethtool and
> > > phylib to resolve PLCA get/set config and get status messages.
> > > 
> > > Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@...il.com>
> > > ---
> > >  drivers/net/phy/phy.c        | 172 +++++++++++++++++++++++++++++++++++
> > >  drivers/net/phy/phy_device.c |   3 +
> > >  include/linux/phy.h          |   7 ++
> > >  3 files changed, 182 insertions(+)
> > 
> > <...>
> > 
> > > +	curr_plca_cfg = kmalloc(sizeof(*curr_plca_cfg), GFP_KERNEL);
> > > +	if (unlikely(!curr_plca_cfg)) {
> > 
> > Please don't put likely/unlikely on kamlloc and/or in in control path
> > flow.
> Fixed. Although, I am curious to know why exactly it is bad to
> linkely/unlikely on kmalloc. Is this because if we're in a situation of
> low memory we don't want to put more "stress" on the system failing
> branch predictions?

likely/unlikely is helpful when you are sure that compiler won't be able
to guess the right flow and probably will get it wrong. It is usually not
the case for any "return to error" flows and sure not the case here.

Bottom line, you won't see any difference in assembly if you put or
remove unlikely keyword.

Thanks

> > 
> > Thanks