On Tue, Dec 18, 2018 at 02:15:46PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav@xxxxxxxxxxxx>
>
> Refactor code to use cdev_device_add() and do other minor refactors
> while modifying these functions as below.
>
> 1. Instead of returning generic -1, return an actual error for ib_umad_init_port().
> 2. Introduce and use ib_umad_init_port_dev() for sm and umad char devices.
> 3. Instead of kobj, use more light weight kref to refcount ib_umad_device.
> 4. Use modern cdev_device_add() single code cut down three steps of
> cdev_add(), device_create(). This further helps to move device sysfs
> files to class attributes in subsequent patch.
> 5. Remove few empty lines while refactoring these functions.
> 6. Use sizeof() instead of sizeof to avoid checkpatch warning.
> 7. Align ib_umad_port fields by tab while changing *dev to dev.
>
> Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx>
> Reviewed-by: Jack Morgenstein <jackm@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> drivers/infiniband/core/user_mad.c | 184 +++++++++++++++--------------
> 1 file changed, 93 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
> index 4558bf8b5ed5..732dd0dbb0dd 100644
> +++ b/drivers/infiniband/core/user_mad.c
> @@ -89,25 +89,24 @@ enum {
> */
>
> struct ib_umad_port {
> - struct cdev cdev;
> - struct device *dev;
> -
> - struct cdev sm_cdev;
> - struct device *sm_dev;
> - struct semaphore sm_sem;
> -
> - struct mutex file_mutex;
> - struct list_head file_list;
> -
> - struct ib_device *ib_dev;
> - struct ib_umad_device *umad_dev;
> - int dev_num;
> - u8 port_num;
> + struct cdev cdev;
> + struct device dev;
> + struct cdev sm_cdev;
> + struct device sm_dev;
> + struct semaphore sm_sem;
> +
> + struct mutex file_mutex;
> + struct list_head file_list;
> +
> + struct ib_device *ib_dev;
> + struct ib_umad_device *umad_dev;
> + int dev_num;
> + u8 port_num;
> };
>
> struct ib_umad_device {
> - struct kobject kobj;
> - struct ib_umad_port port[0];
> + struct kref kref;
> + struct ib_umad_port ports[0];
> };
Yikes that is a lot of krefs in this one allocation..
Drop the horizontal white space
Use c99 flex array syntax (ports[])
> +static void ib_umad_release_port(struct device *device)
> +{
> + /* device_release() requires minimum one release callback
> + * per class or device. So have an empty one.
> + */
> +}
Not empty.. the release function for the struct device should be a
ib_umad_dev_put() on the containing ib_umad_device struct.
> +
> +static void ib_umad_init_port_dev(struct device *dev,
> + struct ib_umad_port *port,
> + const struct ib_device *device)
> +{
> + device_initialize(dev);
> + dev->class = &umad_class;
> + dev->parent = device->dev.parent;
> + dev_set_drvdata(dev, port);
The drvdata should also be converted to use container_of and removed.
> @@ -1284,22 +1289,19 @@ static void ib_umad_add_one(struct ib_device *device)
> s = rdma_start_port(device);
> e = rdma_end_port(device);
>
> - umad_dev = kzalloc(sizeof *umad_dev +
> + umad_dev = kzalloc(sizeof(*umad_dev) +
> (e - s + 1) * sizeof (struct ib_umad_port),
> GFP_KERNEL);
This should be updated to be struct_size(umad_dev, ports, e - s + 1)
Surprised it got missed in the bulk conversion
Jason
