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

[Leon Romanovsky]

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@xxxxxxxxx>
> > > ---
> > >  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