> -----Original Message-----
> From: Jason Gunthorpe
> Sent: Monday, September 17, 2018 4:45 PM
> To: linux-rdma@xxxxxxxxxxxxxxx; Parav Pandit <parav@xxxxxxxxxxxx>
> Subject: [PATCH] RDMA/uverbs: Fix error unwind in ib_uverbs_add_one
>
> The error path has several mistakes
>
> - cdev_del should not be called if cdev_device_add fails
> - We must call put_device on all the goto exit paths
> as that is what frees the uapi, SRCU and the struct itself.
>
> Fixes: c5c4d92e70f3 ("RDMA/uverbs: Use cdev_device_add() instead of
> cdev_add()")
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> ---
> drivers/infiniband/core/uverbs_main.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> Parav, yes?
>
Looks good.
> diff --git a/drivers/infiniband/core/uverbs_main.c
> b/drivers/infiniband/core/uverbs_main.c
> index 16e5f714ca536e..ed8db441adb90e 100644
> --- a/drivers/infiniband/core/uverbs_main.c
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -1031,6 +1031,11 @@ static void ib_uverbs_add_one(struct ib_device
> *device)
> return;
> }
>
> + device_initialize(&uverbs_dev->dev);
> + uverbs_dev->dev.class = uverbs_class;
> + uverbs_dev->dev.parent = device->dev.parent;
> + uverbs_dev->dev.release = ib_uverbs_release_dev;
> +
Can you please move below two lines also here so that most static fields gets initialized together and its easy to read.
uverbs_dev->groups[0] = &dev_attr_group;
uverbs_dev->dev.groups = uverbs_dev->groups;
> atomic_set(&uverbs_dev->refcount, 1);
> init_completion(&uverbs_dev->comp);
> uverbs_dev->xrcd_tree = RB_ROOT;
> @@ -1055,11 +1060,7 @@ static void ib_uverbs_add_one(struct ib_device
> *device)
> if (ib_uverbs_create_uapi(device, uverbs_dev))
> goto err_uapi;
>
> - device_initialize(&uverbs_dev->dev);
> - uverbs_dev->dev.class = uverbs_class;
> - uverbs_dev->dev.parent = device->dev.parent;
> uverbs_dev->dev.devt = base;
> - uverbs_dev->dev.release = ib_uverbs_release_dev;
> uverbs_dev->groups[0] = &dev_attr_group;
> uverbs_dev->dev.groups = uverbs_dev->groups;
> dev_set_name(&uverbs_dev->dev, "uverbs%d", uverbs_dev-
> >devnum); @@ -1070,20 +1071,18 @@ static void ib_uverbs_add_one(struct
> ib_device *device)
>
> ret = cdev_device_add(&uverbs_dev->cdev, &uverbs_dev->dev);
> if (ret)
> - goto err_cdev;
> + goto err_uapi;
>
> ib_set_client_data(device, &uverbs_client, uverbs_dev);
> return;
>
> -err_cdev:
> - cdev_del(&uverbs_dev->cdev);
> - put_device(&uverbs_dev->dev);
> err_uapi:
> clear_bit(devnum, dev_map);
> err:
> if (atomic_dec_and_test(&uverbs_dev->refcount))
> ib_uverbs_comp_dev(uverbs_dev);
> wait_for_completion(&uverbs_dev->comp);
> + put_device(&uverbs_dev->dev);
> return;
> }
>
> --
> 2.19.0
Reviewed-by: parav@xxxxxxxxxxxx
