Use the same technique as gid_attrs now uses to manage the port sysfs. Bundle everything into three allocations and use a single sysfs_create_groups() to build everything in one shot. All the memory is always freed in the kobj release function, removing most of the error unwinding. The gid_attr technique and the hw_counters are very similar, merge the two together and combine the sysfs_create_group() call for hw_counters with the single sysfs group setup. Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> --- drivers/infiniband/core/sysfs.c | 321 ++++++++++---------------------- 1 file changed, 103 insertions(+), 218 deletions(-) diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c index 569e847757b944..53838bce574264 100644 --- a/drivers/infiniband/core/sysfs.c +++ b/drivers/infiniband/core/sysfs.c @@ -79,11 +79,12 @@ struct ib_port { struct kobject kobj; struct ib_device *ibdev; struct gid_attr_group *gid_attr_group; - struct attribute_group gid_group; - struct attribute_group *pkey_group; - const struct attribute_group *pma_table; struct hw_stats_port_data *hw_stats_data; + + struct attribute_group groups[3]; + const struct attribute_group *groups_list[5]; u32 port_num; + struct port_table_attribute attrs_list[]; }; struct hw_stats_device_attribute { @@ -112,7 +113,6 @@ struct hw_stats_device_data { }; struct hw_stats_port_data { - struct attribute_group group; struct rdma_hw_stats *stats; struct hw_stats_port_attribute attrs[]; }; @@ -750,30 +750,16 @@ static const struct attribute_group pma_group_noietf = { static void ib_port_release(struct kobject *kobj) { - struct ib_port *p = container_of(kobj, struct ib_port, kobj); - struct attribute *a; + struct ib_port *port = container_of(kobj, struct ib_port, kobj); int i; - if (p->gid_group.attrs) { - for (i = 0; (a = p->gid_group.attrs[i]); ++i) - kfree(a); - - kfree(p->gid_group.attrs); + for (i = 0; i != ARRAY_SIZE(port->groups); i++) + kfree(port->groups[i].attrs); + if (port->hw_stats_data) { + kfree(port->hw_stats_data->stats); + kfree(port->hw_stats_data); } - - if (p->pkey_group) { - if (p->pkey_group->attrs) { - for (i = 0; (a = p->pkey_group->attrs[i]); ++i) - kfree(a); - - kfree(p->pkey_group->attrs); - } - - kfree(p->pkey_group); - p->pkey_group = NULL; - } - - kfree(p); + kfree(port); } static void ib_port_gid_attr_release(struct kobject *kobj) @@ -798,49 +784,6 @@ static struct kobj_type gid_attr_type = { .release = ib_port_gid_attr_release }; -static struct attribute ** -alloc_group_attrs(ssize_t (*show)(struct ib_port *, - struct port_attribute *, char *buf), - int len) -{ - struct attribute **tab_attr; - struct port_table_attribute *element; - int i; - - tab_attr = kcalloc(1 + len, sizeof(struct attribute *), GFP_KERNEL); - if (!tab_attr) - return NULL; - - for (i = 0; i < len; i++) { - element = kzalloc(sizeof(struct port_table_attribute), - GFP_KERNEL); - if (!element) - goto err; - - if (snprintf(element->name, sizeof(element->name), - "%d", i) >= sizeof(element->name)) { - kfree(element); - goto err; - } - - element->attr.attr.name = element->name; - element->attr.attr.mode = S_IRUGO; - element->attr.show = show; - element->index = i; - sysfs_attr_init(&element->attr.attr); - - tab_attr[i] = &element->attr.attr; - } - - return tab_attr; - -err: - while (--i >= 0) - kfree(tab_attr[i]); - kfree(tab_attr); - return NULL; -} - /* * Figure out which counter table to use depending on * the device capabilities. @@ -1051,7 +994,8 @@ static void destroy_hw_device_stats(struct ib_device *ibdev) ibdev->hw_stats_data = NULL; } -static struct hw_stats_port_data *alloc_hw_stats_port(struct ib_port *port) +static struct hw_stats_port_data * +alloc_hw_stats_port(struct ib_port *port, struct attribute_group *group) { struct ib_device *ibdev = port->ibdev; struct hw_stats_port_data *data; @@ -1073,13 +1017,13 @@ static struct hw_stats_port_data *alloc_hw_stats_port(struct ib_port *port) GFP_KERNEL); if (!data) goto err_free_stats; - data->group.attrs = kcalloc(stats->num_counters + 2, - sizeof(*data->group.attrs), GFP_KERNEL); - if (!data->group.attrs) + group->attrs = kcalloc(stats->num_counters + 2, + sizeof(*group->attrs), GFP_KERNEL); + if (!group->attrs) goto err_free_data; mutex_init(&stats->lock); - data->group.name = "hw_counters"; + group->name = "hw_counters"; data->stats = stats; return data; @@ -1090,20 +1034,14 @@ static struct hw_stats_port_data *alloc_hw_stats_port(struct ib_port *port) return ERR_PTR(-ENOMEM); } -static void free_hw_stats_port(struct hw_stats_port_data *data) -{ - kfree(data->group.attrs); - kfree(data->stats); - kfree(data); -} - -static int setup_hw_port_stats(struct ib_port *port) +static int setup_hw_port_stats(struct ib_port *port, + struct attribute_group *group) { struct hw_stats_port_attribute *attr; struct hw_stats_port_data *data; int i, ret; - data = alloc_hw_stats_port(port); + data = alloc_hw_stats_port(port, group); if (IS_ERR(data)) return PTR_ERR(data); @@ -1112,9 +1050,10 @@ static int setup_hw_port_stats(struct ib_port *port) 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; for (i = 0; i < data->stats->num_counters; i++) { @@ -1124,7 +1063,7 @@ static int setup_hw_port_stats(struct ib_port *port) attr->attr.attr.mode = 0444; attr->attr.show = hw_stat_port_show; attr->show = show_hw_stats; - data->group.attrs[i] = &attr->attr.attr; + group->attrs[i] = &attr->attr.attr; } attr = &data->attrs[i]; @@ -1135,27 +1074,10 @@ static int setup_hw_port_stats(struct ib_port *port) attr->show = show_stats_lifespan; attr->attr.store = hw_stat_port_store; attr->store = set_stats_lifespan; - data->group.attrs[i] = &attr->attr.attr; + group->attrs[i] = &attr->attr.attr; port->hw_stats_data = data; - ret = sysfs_create_group(&port->kobj, &data->group); - if (ret) - goto err_free; return 0; - -err_free: - free_hw_stats_port(data); - port->hw_stats_data = NULL; - return ret; -} - -static void destroy_hw_port_stats(struct ib_port *port) -{ - if (!port->hw_stats_data) - return; - sysfs_remove_group(&port->kobj, &port->hw_stats_data->group); - free_hw_stats_port(port->hw_stats_data); - port->hw_stats_data = NULL; } struct rdma_hw_stats *ib_get_hw_stats_port(struct ib_device *ibdev, @@ -1266,68 +1188,42 @@ static void destroy_gid_attrs(struct ib_port *port) kobject_put(&gid_attr_group->kobj); } -static int add_port(struct ib_core_device *coredev, int port_num) +/* + * Create the sysfs: + * ibp0s9/ports/XX/{gids,pkeys,counters}/YYY + */ +static struct ib_port *setup_port(struct ib_core_device *coredev, int port_num, + const struct ib_port_attr *attr) { struct ib_device *device = rdma_device_to_ibdev(&coredev->dev); bool is_full_dev = &device->coredev == coredev; + const struct attribute_group **cur_group; struct ib_port *p; - struct ib_port_attr attr; - int i; int ret; - ret = ib_query_port(device, port_num, &attr); - if (ret) - return ret; - - p = kzalloc(sizeof *p, GFP_KERNEL); + p = kzalloc(struct_size(p, attrs_list, + attr->gid_tbl_len + attr->pkey_tbl_len), + GFP_KERNEL); if (!p) - return -ENOMEM; - - p->ibdev = device; - p->port_num = port_num; + return ERR_PTR(-ENOMEM); + p->ibdev = device; + p->port_num = port_num; + kobject_init(&p->kobj, &port_type); - ret = kobject_init_and_add(&p->kobj, &port_type, - coredev->ports_kobj, - "%d", port_num); + cur_group = p->groups_list; + ret = alloc_port_table_group("gids", &p->groups[0], p->attrs_list, + attr->gid_tbl_len, show_port_gid); if (ret) goto err_put; + *cur_group++ = &p->groups[0]; - if (device->ops.process_mad && is_full_dev) { - p->pma_table = get_counter_table(device, port_num); - ret = sysfs_create_group(&p->kobj, p->pma_table); + if (attr->pkey_tbl_len) { + ret = alloc_port_table_group("pkeys", &p->groups[1], + p->attrs_list + attr->gid_tbl_len, + attr->pkey_tbl_len, show_port_pkey); if (ret) goto err_put; - } - - p->gid_group.name = "gids"; - p->gid_group.attrs = alloc_group_attrs(show_port_gid, attr.gid_tbl_len); - if (!p->gid_group.attrs) { - ret = -ENOMEM; - goto err_remove_pma; - } - - ret = sysfs_create_group(&p->kobj, &p->gid_group); - if (ret) - goto err_free_gid; - - if (attr.pkey_tbl_len) { - p->pkey_group = kzalloc(sizeof(*p->pkey_group), GFP_KERNEL); - if (!p->pkey_group) { - ret = -ENOMEM; - goto err_remove_gid; - } - - p->pkey_group->name = "pkeys"; - p->pkey_group->attrs = alloc_group_attrs(show_port_pkey, - attr.pkey_tbl_len); - if (!p->pkey_group->attrs) { - ret = -ENOMEM; - goto err_free_pkey_group; - } - - ret = sysfs_create_group(&p->kobj, p->pkey_group); - if (ret) - goto err_free_pkey; + *cur_group++ = &p->groups[1]; } /* @@ -1336,66 +1232,45 @@ static int add_port(struct ib_core_device *coredev, int port_num) * counter initialization. */ if (port_num && is_full_dev) { - ret = setup_hw_port_stats(p); + ret = setup_hw_port_stats(p, &p->groups[2]); if (ret && ret != -EOPNOTSUPP) - goto err_remove_pkey; + goto err_put; + if (!ret) + *cur_group++ = &p->groups[2]; } - ret = setup_gid_attrs(p, &attr); - if (ret) - goto err_remove_stats; - if (device->ops.init_port && is_full_dev) { - ret = device->ops.init_port(device, port_num, &p->kobj); - if (ret) - goto err_remove_gid_attrs; - } + if (device->ops.process_mad && is_full_dev) + *cur_group++ = get_counter_table(device, port_num); + + ret = kobject_add(&p->kobj, coredev->ports_kobj, "%d", port_num); + if (ret) + goto err_put; + ret = sysfs_create_groups(&p->kobj, p->groups_list); + if (ret) + goto err_del; list_add_tail(&p->kobj.entry, &coredev->port_list); if (device->port_data && is_full_dev) device->port_data[port_num].sysfs = p; - kobject_uevent(&p->kobj, KOBJ_ADD); - return 0; - -err_remove_gid_attrs: - destroy_gid_attrs(p); - -err_remove_stats: - destroy_hw_port_stats(p); - -err_remove_pkey: - if (p->pkey_group) - sysfs_remove_group(&p->kobj, p->pkey_group); - -err_free_pkey: - if (p->pkey_group) { - for (i = 0; i < attr.pkey_tbl_len; ++i) - kfree(p->pkey_group->attrs[i]); - - kfree(p->pkey_group->attrs); - p->pkey_group->attrs = NULL; - } - -err_free_pkey_group: - kfree(p->pkey_group); - -err_remove_gid: - sysfs_remove_group(&p->kobj, &p->gid_group); - -err_free_gid: - for (i = 0; i < attr.gid_tbl_len; ++i) - kfree(p->gid_group.attrs[i]); - - kfree(p->gid_group.attrs); - p->gid_group.attrs = NULL; - -err_remove_pma: - if (p->pma_table) - sysfs_remove_group(&p->kobj, p->pma_table); + return p; +err_del: + kobject_del(&p->kobj); err_put: kobject_put(&p->kobj); - return ret; + return ERR_PTR(ret); +} + +static void destroy_port(struct ib_port *port) +{ + if (port->ibdev->port_data && + port->ibdev->port_data[port->port_num].sysfs == port) + port->ibdev->port_data[port->port_num].sysfs = NULL; + list_del(&port->kobj.entry); + sysfs_remove_groups(&port->kobj, port->groups_list); + kobject_del(&port->kobj); + kobject_put(&port->kobj); } static const char *node_type_string(int node_type) @@ -1512,25 +1387,13 @@ const struct attribute_group ib_dev_attr_group = { void ib_free_port_attrs(struct ib_core_device *coredev) { - struct ib_device *device = rdma_device_to_ibdev(&coredev->dev); - bool is_full_dev = &device->coredev == coredev; struct kobject *p, *t; list_for_each_entry_safe(p, t, &coredev->port_list, entry) { struct ib_port *port = container_of(p, struct ib_port, kobj); - list_del(&p->entry); - destroy_hw_port_stats(port); - if (device->port_data && is_full_dev) - device->port_data[port->port_num].sysfs = NULL; - - if (port->pma_table) - sysfs_remove_group(p, port->pma_table); - if (port->pkey_group) - sysfs_remove_group(p, port->pkey_group); - sysfs_remove_group(p, &port->gid_group); destroy_gid_attrs(port); - kobject_put(p); + destroy_port(port); } kobject_put(coredev->ports_kobj); @@ -1539,7 +1402,8 @@ void ib_free_port_attrs(struct ib_core_device *coredev) int ib_setup_port_attrs(struct ib_core_device *coredev) { struct ib_device *device = rdma_device_to_ibdev(&coredev->dev); - u32 port; + bool is_full_dev = &device->coredev == coredev; + u32 port_num; int ret; coredev->ports_kobj = kobject_create_and_add("ports", @@ -1547,12 +1411,33 @@ int ib_setup_port_attrs(struct ib_core_device *coredev) if (!coredev->ports_kobj) return -ENOMEM; - rdma_for_each_port (device, port) { - ret = add_port(coredev, port); + rdma_for_each_port (device, port_num) { + struct ib_port_attr attr; + struct ib_port *port; + + ret = ib_query_port(device, port_num, &attr); + if (ret) + goto err_put; + + port = setup_port(coredev, port_num, &attr); + if (IS_ERR(port)) { + ret = PTR_ERR(port); + goto err_put; + } + + ret = setup_gid_attrs(port, &attr); if (ret) goto err_put; - } + if (device->ops.init_port && is_full_dev) { + ret = device->ops.init_port(device, port_num, + &port->kobj); + if (ret) + goto err_put; + } + + kobject_uevent(&port->kobj, KOBJ_ADD); + } return 0; err_put: -- 2.31.1