[PATCH rdma-next v2 07/15] RDMA/core: Create the device hw_counters through the normal groups mechanism

From: Leon Romanovsky
Date: Fri Jun 11 2021 - 12:01:19 EST


From: Jason Gunthorpe <jgg@xxxxxxxxxx>

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.

Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
---
drivers/infiniband/core/core_priv.h | 4 +-
drivers/infiniband/core/device.c | 11 ++++-
drivers/infiniband/core/sysfs.c | 66 +++++++----------------------
include/rdma/ib_verbs.h | 9 ++--
4 files changed, 32 insertions(+), 58 deletions(-)

diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index ec5c2c3db423..6066c4b39876 100644
--- 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 86a16cd7d7fd..030a4041b2e0 100644
--- 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 @@ int ib_register_device(struct ib_device *device, const char *name,
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 2631c179e004..07a00d3d3d44 100644
--- 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 @@ alloc_hw_stats_device(struct ib_device *ibdev)
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 @@ int ib_setup_port_attrs(struct ib_core_device *coredev)
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 0dc7ab1a8dcf..5ca1cb82a543 100644
--- 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;

--
2.31.1