On Thu, Jul 06, 2017 at 09:40:04AM -0400, Doug Ledford wrote:
> On 7/6/2017 12:23 AM, Leon Romanovsky wrote:
> > On Wed, Jul 05, 2017 at 05:17:52PM -0400, Doug Ledford wrote:
> > > From: Niranjana Vishwanathapura <niranjana.vishwanathapura@xxxxxxxx
> > > m>
> > >
> > > -static void mlx5_ib_free_rdma_netdev(struct net_device *netdev)
> > > -{
> > > - return mlx5_rdma_netdev_free(netdev);
> > > + netdev = mlx5_rdma_netdev_alloc(to_mdev(hca)->mdev, hca,
> > > + name, setup);
> > > + if (likely(!IS_ERR_OR_NULL(netdev))) {
> > > + rn = netdev_priv(netdev);
> > > + rn->free_rdma_netdev = mlx5_ib_free_rdma_netdev;
> > > + }
> > > + return netdev;
> > > }
> >
> >
> > Thanks Doug, it looks good enough for the fix.
> >
> > In general, the "likely" is not needed here (we are not in data path)
>
> It doesn't hurt though...
>
> > and our preference is to avoid "if(!error) { do something }"
> > constructions
> > in favor of "if(error) { return ...}" (fail as early as you can).
>
> Normally I would agree with you on that point. But when you aren't
> returning an error code but instead are returning the same thing you
> return in the non error case, and when there are so few things to be
> done in the non error case, I think this sort of construct becomes more
> appealing (mainly because it will more closely match the assembler that
> GCC puts out when compiling this code and I think that has value for
> those times when you need to debug the object code, but that's just my
> personal opinion).
>
> > Reviewed-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
Any options are good for me, it fixes the crash :)
Thanks
>
>
> --
> Doug Ledford <dledford@xxxxxxxxxx>
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
>
Attachment:
signature.asc
Description: PGP signature
