Instead of having an whole bunch of different allocations to create the gid_attr kobjects reduce it to three, one for the kobj struct plus the attributes, and one for the attribute list for each of the two groups. Move the freeing of all allocations to the release function. Reorder the operations so all the allocations happen first then the kobject & sysfs operations are last. This removes the majority of the complicated error unwind since the release function will always undo all the memory allocations. Freeing the memory is also much simpler since there is a lot less of it. Consolidate creating the "group of array indexes" pattern into one helper function. Ensure kobject_del is used. Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> --- drivers/infiniband/core/sysfs.c | 171 +++++++++++++++++--------------- 1 file changed, 90 insertions(+), 81 deletions(-) diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c index d2a089a6f66639..569e847757b944 100644 --- a/drivers/infiniband/core/sysfs.c +++ b/drivers/infiniband/core/sysfs.c @@ -47,23 +47,6 @@ struct ib_port; -struct gid_attr_group { - struct ib_port *port; - struct kobject kobj; - struct attribute_group ndev; - struct attribute_group type; -}; -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; - u32 port_num; -}; - struct port_attribute { struct attribute attr; ssize_t (*show)(struct ib_port *, struct port_attribute *, char *buf); @@ -84,6 +67,25 @@ struct port_table_attribute { __be16 attr_id; }; +struct gid_attr_group { + struct ib_port *port; + struct kobject kobj; + struct attribute_group groups[2]; + const struct attribute_group *groups_list[3]; + struct port_table_attribute attrs_list[]; +}; + +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; + u32 port_num; +}; + struct hw_stats_device_attribute { struct device_attribute attr; ssize_t (*show)(struct ib_device *ibdev, struct rdma_hw_stats *stats, @@ -776,26 +778,13 @@ static void ib_port_release(struct kobject *kobj) static void ib_port_gid_attr_release(struct kobject *kobj) { - struct gid_attr_group *g = container_of(kobj, struct gid_attr_group, - kobj); - struct attribute *a; + struct gid_attr_group *gid_attr_group = + container_of(kobj, struct gid_attr_group, kobj); int i; - if (g->ndev.attrs) { - for (i = 0; (a = g->ndev.attrs[i]); ++i) - kfree(a); - - kfree(g->ndev.attrs); - } - - if (g->type.attrs) { - for (i = 0; (a = g->type.attrs[i]); ++i) - kfree(a); - - kfree(g->type.attrs); - } - - kfree(g); + for (i = 0; i != ARRAY_SIZE(gid_attr_group->groups); i++) + kfree(gid_attr_group->groups[i].attrs); + kfree(gid_attr_group); } static struct kobj_type port_type = { @@ -1178,6 +1167,42 @@ struct rdma_hw_stats *ib_get_hw_stats_port(struct ib_device *ibdev, return ibdev->port_data[port_num].sysfs->hw_stats_data->stats; } +static int alloc_port_table_group( + const char *name, struct attribute_group *group, + struct port_table_attribute *attrs, size_t num, + ssize_t (*show)(struct ib_port *, struct port_attribute *, char *buf)) +{ + struct attribute **attr_list; + int i; + + attr_list = kcalloc(num + 1, sizeof(*attr_list), GFP_KERNEL); + if (!attr_list) + return -ENOMEM; + + for (i = 0; i < num; i++) { + struct port_table_attribute *element = &attrs[i]; + + if (snprintf(element->name, sizeof(element->name), "%d", i) >= + sizeof(element->name)) { + goto err; + } + + sysfs_attr_init(&element->attr.attr); + element->attr.attr.name = element->name; + element->attr.attr.mode = 0444; + element->attr.show = show; + element->index = i; + + attr_list[i] = &element->attr.attr; + } + group->name = name; + group->attrs = attr_list; + return 0; +err: + kfree(attr_list); + return -EINVAL; +} + /* * Create the sysfs: * ibp0s9/ports/XX/gid_attrs/{ndevs,types}/YYY @@ -1188,60 +1213,44 @@ static int setup_gid_attrs(struct ib_port *port, { struct gid_attr_group *gid_attr_group; int ret; - int i; - gid_attr_group = kzalloc(sizeof(*gid_attr_group), GFP_KERNEL); + gid_attr_group = kzalloc(struct_size(gid_attr_group, attrs_list, + attr->gid_tbl_len * 2), + GFP_KERNEL); if (!gid_attr_group) return -ENOMEM; - gid_attr_group->port = port; - ret = kobject_init_and_add(&gid_attr_group->kobj, &gid_attr_type, - &port->kobj, "gid_attrs"); - if (ret) - goto err_put_gid_attrs; - - gid_attr_group->ndev.name = "ndevs"; - gid_attr_group->ndev.attrs = - alloc_group_attrs(show_port_gid_attr_ndev, attr->gid_tbl_len); - if (!gid_attr_group->ndev.attrs) { - ret = -ENOMEM; - goto err_put_gid_attrs; - } + kobject_init(&gid_attr_group->kobj, &gid_attr_type); - ret = sysfs_create_group(&gid_attr_group->kobj, &gid_attr_group->ndev); + ret = alloc_port_table_group("ndevs", &gid_attr_group->groups[0], + gid_attr_group->attrs_list, + attr->gid_tbl_len, + show_port_gid_attr_ndev); if (ret) - goto err_free_gid_ndev; - - gid_attr_group->type.name = "types"; - gid_attr_group->type.attrs = alloc_group_attrs( - show_port_gid_attr_gid_type, attr->gid_tbl_len); - if (!gid_attr_group->type.attrs) { - ret = -ENOMEM; - goto err_remove_gid_ndev; - } + goto err_put; + gid_attr_group->groups_list[0] = &gid_attr_group->groups[0]; - ret = sysfs_create_group(&gid_attr_group->kobj, &gid_attr_group->type); + ret = alloc_port_table_group( + "types", &gid_attr_group->groups[1], + gid_attr_group->attrs_list + attr->gid_tbl_len, + attr->gid_tbl_len, show_port_gid_attr_gid_type); if (ret) - goto err_free_gid_type; + goto err_put; + gid_attr_group->groups_list[1] = &gid_attr_group->groups[1]; + ret = kobject_add(&gid_attr_group->kobj, &port->kobj, "gid_attrs"); + if (ret) + goto err_put; + ret = sysfs_create_groups(&gid_attr_group->kobj, + gid_attr_group->groups_list); + if (ret) + goto err_del; port->gid_attr_group = gid_attr_group; return 0; -err_free_gid_type: - for (i = 0; i < attr->gid_tbl_len; ++i) - kfree(gid_attr_group->type.attrs[i]); - - kfree(gid_attr_group->type.attrs); - gid_attr_group->type.attrs = NULL; -err_remove_gid_ndev: - sysfs_remove_group(&gid_attr_group->kobj, &gid_attr_group->ndev); -err_free_gid_ndev: - for (i = 0; i < attr->gid_tbl_len; ++i) - kfree(gid_attr_group->ndev.attrs[i]); - - kfree(gid_attr_group->ndev.attrs); - gid_attr_group->ndev.attrs = NULL; -err_put_gid_attrs: +err_del: + kobject_del(&gid_attr_group->kobj); +err_put: kobject_put(&gid_attr_group->kobj); return ret; } @@ -1250,10 +1259,10 @@ static void destroy_gid_attrs(struct ib_port *port) { struct gid_attr_group *gid_attr_group = port->gid_attr_group; - sysfs_remove_group(&gid_attr_group->kobj, - &gid_attr_group->ndev); - sysfs_remove_group(&gid_attr_group->kobj, - &gid_attr_group->type); + if (!gid_attr_group) + return; + sysfs_remove_groups(&gid_attr_group->kobj, gid_attr_group->groups_list); + kobject_del(&gid_attr_group->kobj); kobject_put(&gid_attr_group->kobj); } -- 2.31.1