Re: [PATCH for-rc v1] IB/core, opa_vnic, hfi1, mlx5: Properly free rdma_netdev

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

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-ral2JQCrhuE@public.gmane.org
> > > 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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >

Any options are good for me, it fixes the crash :)

Thanks

>
>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG KeyID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]