> On 12 Feb 2019, at 14:11, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> 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.
>
"ib_dealloc_device(&ibdev->ib_dev);" makes sense, hence:
Reviewed-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx>
Thxs, Håkon
> Thanks
>
>>
>>
>> Thxs, Håkon
>>
>>
>>
>>>>> return -EINVAL;
>>>>> + }
>>>>>
>>>>
>>>> This is wrong :/
>>>>
>>>
>>> Thanks,
>>
