On Tue, Feb 12, 2019 at 12:46:02PM +0100, Håkon Bugge wrote:
>
>
> > On 12 Feb 2019, at 07:16, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
> >
> > On Mon, Feb 11, 2019 at 09:12:03PM +0000, Mark Bloch wrote:
> >>
> >>
> >> On 2/11/2019 07:40, Leon Romanovsky wrote:
> >>> From: Mark Bloch <markb@xxxxxxxxxxxx>
> >>>
> >>> Make sure the IB device is freed on failure.
> >>>
> >>> Fixes: b5ca15ad7e61 ("IB/mlx5: Add proper representors support")
> >>> Signed-off-by: Mark Bloch <markb@xxxxxxxxxxxx>
> >>> Reviewed-by: Bodong Wang <bodong@xxxxxxxxxxxx>
> >>> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >>> ---
> >>> drivers/infiniband/hw/mlx5/ib_rep.c | 4 +++-
> >>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c b/drivers/infiniband/hw/mlx5/ib_rep.c
> >>> index 6d7b8bad4b61..ff663e0ec3c2 100644
> >>> --- a/drivers/infiniband/hw/mlx5/ib_rep.c
> >>> +++ b/drivers/infiniband/hw/mlx5/ib_rep.c
> >>> @@ -78,8 +78,10 @@ mlx5_ib_vport_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
> >>> ibdev->mdev = dev;
> >>> ibdev->num_ports = max(MLX5_CAP_GEN(dev, num_ports),
> >>> MLX5_CAP_GEN(dev, num_vhca_ports));
> >>> - if (!__mlx5_ib_add(ibdev, &rep_profile))
> >>> + if (!__mlx5_ib_add(ibdev, &rep_profile)) {
> >>> + ib_dealloc_device(&dev->ib_dev);
>
> I cannot see where/how ibdev has propagated into dev here, in particular when __mlx5_ib_add() failed.
>
> Isn't more intuitive to dealloc the pointer that was just alloc'd, i.e.:
> + ib_dealloc_device(ibdev);
>
> ?
Intuitive? Yes, but unfortunately it is not how RDMA subsystem is
designed. All driver objects and devices are "boxes" for core structs.
In our case ib_dealloc_device() receives "struct ib_device *" as an
input and not "struct mlx5_ib_dev *".
The final code is "ib_dealloc_device(&ibdev->ib_dev);" and not
"ib_dealloc_device(&dev->ib_dev);" as I sent.
Thanks
>
>
> Thxs, Håkon
>
>
>
> >>> return -EINVAL;
> >>> + }
> >>>
> >>
> >> This is wrong :/
> >>
> >
> > Thanks,
>
Attachment:
signature.asc
Description: PGP signature
