RDMA/core: Create the device hw_counters through the normal groups mechanism

Instead of calling device_add_groups() add the group to the existing
groups array which is managed through device_add().

This requires setting up the hw_counters before device_add(), so it gets
split up from the already split port sysfs flow.

Move all the memory freeing to the release function.

Link: https://lore.kernel.org/r/666250d937b64f6fdf45da9e2dc0b6e5e4f7abd8.1623427137.git.leonro@nvidia.com
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index ec5c2c3..6066c4b 100644 (file)
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -78,8 +78,6 @@ static inline struct rdma_dev_net *rdma_net_to_dev_net(struct net *net)
        return net_generic(net, rdma_dev_net_id);
 }
 
-int ib_device_register_sysfs(struct ib_device *device);
-void ib_device_unregister_sysfs(struct ib_device *device);
 int ib_device_rename(struct ib_device *ibdev, const char *name);
 int ib_device_set_dim(struct ib_device *ibdev, u8 use_dim);
 
@@ -379,6 +377,8 @@ struct net_device *rdma_read_gid_attr_ndev_rcu(const struct ib_gid_attr *attr);
 void ib_free_port_attrs(struct ib_core_device *coredev);
 int ib_setup_port_attrs(struct ib_core_device *coredev);
 struct rdma_hw_stats *ib_get_hw_stats_port(struct ib_device *ibdev, u32 port_num);
+void ib_device_release_hw_stats(struct hw_stats_device_data *data);
+int ib_setup_device_attrs(struct ib_device *ibdev);
 
 int rdma_compatdev_set(u8 enable);
 

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 86a16cd..030a404 100644 (file)
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -491,6 +491,8 @@ static void ib_device_release(struct device *device)
 
        free_netdevs(dev);
        WARN_ON(refcount_read(&dev->refcount));
+       if (dev->hw_stats_data)
+               ib_device_release_hw_stats(dev->hw_stats_data);
        if (dev->port_data) {
                ib_cache_release_one(dev);
                ib_security_release_port_pkey_list(dev);
@@ -1394,6 +1396,10 @@ int ib_register_device(struct ib_device *device, const char *name,
                return ret;
        }
 
+       ret = ib_setup_device_attrs(device);
+       if (ret)
+               goto cache_cleanup;
+
        ib_device_register_rdmacg(device);
 
        rdma_counter_init(device);
@@ -1407,7 +1413,7 @@ int ib_register_device(struct ib_device *device, const char *name,
        if (ret)
                goto cg_cleanup;
 
-       ret = ib_device_register_sysfs(device);
+       ret = ib_setup_port_attrs(&device->coredev);
        if (ret) {
                dev_warn(&device->dev,
                         "Couldn't register device with driver model\n");
@@ -1449,6 +1455,7 @@ dev_cleanup:
 cg_cleanup:
        dev_set_uevent_suppress(&device->dev, false);
        ib_device_unregister_rdmacg(device);
+cache_cleanup:
        ib_cache_cleanup_one(device);
        return ret;
 }
@@ -1473,7 +1480,7 @@ static void __ib_unregister_device(struct ib_device *ib_dev)
        /* Expedite removing unregistered pointers from the hash table */
        free_netdevs(ib_dev);
 
-       ib_device_unregister_sysfs(ib_dev);
+       ib_free_port_attrs(&ib_dev->coredev);
        device_del(&ib_dev->dev);
        ib_device_unregister_rdmacg(ib_dev);
        ib_cache_cleanup_one(ib_dev);

diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index 2631c17..07a00d3 100644 (file)
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -107,7 +107,6 @@ struct hw_stats_port_attribute {
 
 struct hw_stats_device_data {
        struct attribute_group group;
-       const struct attribute_group *groups[2];
        struct rdma_hw_stats *stats;
        struct hw_stats_device_attribute attrs[];
 };
@@ -915,7 +914,6 @@ alloc_hw_stats_device(struct ib_device *ibdev)
        mutex_init(&stats->lock);
        data->group.name = "hw_counters";
        data->stats = stats;
-       data->groups[0] = &data->group;
        return data;
 
 err_free_data:
@@ -925,29 +923,33 @@ err_free_stats:
        return ERR_PTR(-ENOMEM);
 }
 
-static void free_hw_stats_device(struct hw_stats_device_data *data)
+void ib_device_release_hw_stats(struct hw_stats_device_data *data)
 {
        kfree(data->group.attrs);
        kfree(data->stats);
        kfree(data);
 }
 
-static int setup_hw_device_stats(struct ib_device *ibdev)
+int ib_setup_device_attrs(struct ib_device *ibdev)
 {
        struct hw_stats_device_attribute *attr;
        struct hw_stats_device_data *data;
        int i, ret;
 
        data = alloc_hw_stats_device(ibdev);
-       if (IS_ERR(data))
+       if (IS_ERR(data)) {
+               if (PTR_ERR(data) == -EOPNOTSUPP)
+                       return 0;
                return PTR_ERR(data);
+       }
+       ibdev->hw_stats_data = data;
 
        ret = ibdev->ops.get_hw_stats(ibdev, data->stats, 0,
                                      data->stats->num_counters);
        if (ret != data->stats->num_counters) {
                if (WARN_ON(ret >= 0))
-                       ret = -EINVAL;
-               goto err_free;
+                       return -EINVAL;
+               return ret;
        }
 
        data->stats->timestamp = jiffies;
@@ -971,26 +973,13 @@ static int setup_hw_device_stats(struct ib_device *ibdev)
        attr->attr.store = hw_stat_device_store;
        attr->store = set_stats_lifespan;
        data->group.attrs[i] = &attr->attr.attr;
-
-       ibdev->hw_stats_data = data;
-       ret = device_add_groups(&ibdev->dev, data->groups);
-       if (ret)
-               goto err_free;
-       return 0;
-
-err_free:
-       free_hw_stats_device(data);
-       ibdev->hw_stats_data = NULL;
-       return ret;
-}
-
-static void destroy_hw_device_stats(struct ib_device *ibdev)
-{
-       if (!ibdev->hw_stats_data)
-               return;
-       device_remove_groups(&ibdev->dev, ibdev->hw_stats_data->groups);
-       free_hw_stats_device(ibdev->hw_stats_data);
-       ibdev->hw_stats_data = NULL;
+       for (i = 0; i != ARRAY_SIZE(ibdev->groups); i++)
+               if (!ibdev->groups[i]) {
+                       ibdev->groups[i] = &data->group;
+                       return 0;
+               }
+       WARN(true, "struct ib_device->groups is too small");
+       return -EINVAL;
 }
 
 static struct hw_stats_port_data *
@@ -1443,29 +1432,6 @@ err_put:
        return ret;
 }
 
-int ib_device_register_sysfs(struct ib_device *device)
-{
-       int ret;
-
-       ret = ib_setup_port_attrs(&device->coredev);
-       if (ret)
-               return ret;
-
-       ret = setup_hw_device_stats(device);
-       if (ret && ret != -EOPNOTSUPP) {
-               ib_free_port_attrs(&device->coredev);
-               return ret;
-       }
-
-       return 0;
-}
-
-void ib_device_unregister_sysfs(struct ib_device *device)
-{
-       destroy_hw_device_stats(device);
-       ib_free_port_attrs(&device->coredev);
-}
-
 /**
  * ib_port_register_module_stat - add module counters under relevant port
  *  of IB device.

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 0dc7ab1..5ca1cb8 100644 (file)
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2677,11 +2677,12 @@ struct ib_device {
                struct ib_core_device   coredev;
        };
 
-       /* First group for device attributes,
-        * Second group for driver provided attributes (optional).
-        * It is NULL terminated array.
+       /* First group is for device attributes,
+        * Second group is for driver provided attributes (optional).
+        * Third group is for the hw_stats
+        * It is a NULL terminated array.
         */
-       const struct attribute_group    *groups[3];
+       const struct attribute_group    *groups[4];
 
        u64                          uverbs_cmd_mask;