From f89ab8619e5320cc9c2576f5f8dcbaf6c0ba3950 Mon Sep 17 00:00:00 2001 From: Joel Becker Date: Thu, 17 Jul 2008 14:53:48 -0700 Subject: Revert "configfs: Allow ->make_item() and ->make_group() to return detailed errors." This reverts commit 11c3b79218390a139f2d474ee1e983a672d5839a. The code will move to PTR_ERR(). Signed-off-by: Joel Becker --- Documentation/filesystems/configfs/configfs.txt | 10 ++--- .../filesystems/configfs/configfs_example.c | 14 +++---- drivers/net/netconsole.c | 10 ++--- fs/configfs/dir.c | 13 ++++--- fs/dlm/config.c | 45 ++++++++-------------- fs/ocfs2/cluster/heartbeat.c | 17 ++++---- fs/ocfs2/cluster/nodemanager.c | 45 ++++++++-------------- include/linux/configfs.h | 4 +- 8 files changed, 64 insertions(+), 94 deletions(-) diff --git a/Documentation/filesystems/configfs/configfs.txt b/Documentation/filesystems/configfs/configfs.txt index 15838d706ea2..44c97e6accb2 100644 --- a/Documentation/filesystems/configfs/configfs.txt +++ b/Documentation/filesystems/configfs/configfs.txt @@ -233,12 +233,10 @@ accomplished via the group operations specified on the group's config_item_type. struct configfs_group_operations { - int (*make_item)(struct config_group *group, - const char *name, - struct config_item **new_item); - int (*make_group)(struct config_group *group, - const char *name, - struct config_group **new_group); + struct config_item *(*make_item)(struct config_group *group, + const char *name); + struct config_group *(*make_group)(struct config_group *group, + const char *name); int (*commit_item)(struct config_item *item); void (*disconnect_notify)(struct config_group *group, struct config_item *item); diff --git a/Documentation/filesystems/configfs/configfs_example.c b/Documentation/filesystems/configfs/configfs_example.c index 0b422acd470c..25151fd5c2c6 100644 --- a/Documentation/filesystems/configfs/configfs_example.c +++ b/Documentation/filesystems/configfs/configfs_example.c @@ -273,13 +273,13 @@ static inline struct simple_children *to_simple_children(struct config_item *ite return item ? container_of(to_config_group(item), struct simple_children, group) : NULL; } -static int simple_children_make_item(struct config_group *group, const char *name, struct config_item **new_item) +static struct config_item *simple_children_make_item(struct config_group *group, const char *name) { struct simple_child *simple_child; simple_child = kzalloc(sizeof(struct simple_child), GFP_KERNEL); if (!simple_child) - return -ENOMEM; + return NULL; config_item_init_type_name(&simple_child->item, name, @@ -287,8 +287,7 @@ static int simple_children_make_item(struct config_group *group, const char *nam simple_child->storeme = 0; - *new_item = &simple_child->item; - return 0; + return &simple_child->item; } static struct configfs_attribute simple_children_attr_description = { @@ -360,21 +359,20 @@ static struct configfs_subsystem simple_children_subsys = { * children of its own. */ -static int group_children_make_group(struct config_group *group, const char *name, struct config_group **new_group) +static struct config_group *group_children_make_group(struct config_group *group, const char *name) { struct simple_children *simple_children; simple_children = kzalloc(sizeof(struct simple_children), GFP_KERNEL); if (!simple_children) - return -ENOMEM; + return NULL; config_group_init_type_name(&simple_children->group, name, &simple_children_type); - *new_group = &simple_children->group; - return 0; + return &simple_children->group; } static struct configfs_attribute group_children_attr_description = { diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 387a13395015..665341e43055 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -585,9 +585,8 @@ static struct config_item_type netconsole_target_type = { * Group operations and type for netconsole_subsys. */ -static int make_netconsole_target(struct config_group *group, - const char *name, - struct config_item **new_item) +static struct config_item *make_netconsole_target(struct config_group *group, + const char *name) { unsigned long flags; struct netconsole_target *nt; @@ -599,7 +598,7 @@ static int make_netconsole_target(struct config_group *group, nt = kzalloc(sizeof(*nt), GFP_KERNEL); if (!nt) { printk(KERN_ERR "netconsole: failed to allocate memory\n"); - return -ENOMEM; + return NULL; } nt->np.name = "netconsole"; @@ -616,8 +615,7 @@ static int make_netconsole_target(struct config_group *group, list_add(&nt->list, &target_list); spin_unlock_irqrestore(&target_list_lock, flags); - *new_item = &nt->item; - return 0; + return &nt->item; } static void drop_netconsole_target(struct config_group *group, diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 0e64312a084c..614e382a6049 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -1073,24 +1073,25 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) group = NULL; item = NULL; if (type->ct_group_ops->make_group) { - ret = type->ct_group_ops->make_group(to_config_group(parent_item), name, &group); - if (!ret) { + group = type->ct_group_ops->make_group(to_config_group(parent_item), name); + if (group) { link_group(to_config_group(parent_item), group); item = &group->cg_item; } } else { - ret = type->ct_group_ops->make_item(to_config_group(parent_item), name, &item); - if (!ret) + item = type->ct_group_ops->make_item(to_config_group(parent_item), name); + if (item) link_obj(parent_item, item); } mutex_unlock(&subsys->su_mutex); kfree(name); - if (ret) { + if (!item) { /* - * If ret != 0, then link_obj() was never called. + * If item == NULL, then link_obj() was never called. * There are no extra references to clean up. */ + ret = -ENOMEM; goto out_put; } diff --git a/fs/dlm/config.c b/fs/dlm/config.c index 492d8caaaf25..eac23bd288b2 100644 --- a/fs/dlm/config.c +++ b/fs/dlm/config.c @@ -41,20 +41,16 @@ struct comm; struct nodes; struct node; -static int make_cluster(struct config_group *, const char *, - struct config_group **); +static struct config_group *make_cluster(struct config_group *, const char *); static void drop_cluster(struct config_group *, struct config_item *); static void release_cluster(struct config_item *); -static int make_space(struct config_group *, const char *, - struct config_group **); +static struct config_group *make_space(struct config_group *, const char *); static void drop_space(struct config_group *, struct config_item *); static void release_space(struct config_item *); -static int make_comm(struct config_group *, const char *, - struct config_item **); +static struct config_item *make_comm(struct config_group *, const char *); static void drop_comm(struct config_group *, struct config_item *); static void release_comm(struct config_item *); -static int make_node(struct config_group *, const char *, - struct config_item **); +static struct config_item *make_node(struct config_group *, const char *); static void drop_node(struct config_group *, struct config_item *); static void release_node(struct config_item *); @@ -396,8 +392,8 @@ static struct node *to_node(struct config_item *i) return i ? container_of(i, struct node, item) : NULL; } -static int make_cluster(struct config_group *g, const char *name, - struct config_group **new_g) +static struct config_group *make_cluster(struct config_group *g, + const char *name) { struct cluster *cl = NULL; struct spaces *sps = NULL; @@ -435,15 +431,14 @@ static int make_cluster(struct config_group *g, const char *name, space_list = &sps->ss_group; comm_list = &cms->cs_group; - *new_g = &cl->group; - return 0; + return &cl->group; fail: kfree(cl); kfree(gps); kfree(sps); kfree(cms); - return -ENOMEM; + return NULL; } static void drop_cluster(struct config_group *g, struct config_item *i) @@ -471,8 +466,7 @@ static void release_cluster(struct config_item *i) kfree(cl); } -static int make_space(struct config_group *g, const char *name, - struct config_group **new_g) +static struct config_group *make_space(struct config_group *g, const char *name) { struct space *sp = NULL; struct nodes *nds = NULL; @@ -495,14 +489,13 @@ static int make_space(struct config_group *g, const char *name, INIT_LIST_HEAD(&sp->members); mutex_init(&sp->members_lock); sp->members_count = 0; - *new_g = &sp->group; - return 0; + return &sp->group; fail: kfree(sp); kfree(gps); kfree(nds); - return -ENOMEM; + return NULL; } static void drop_space(struct config_group *g, struct config_item *i) @@ -529,21 +522,19 @@ static void release_space(struct config_item *i) kfree(sp); } -static int make_comm(struct config_group *g, const char *name, - struct config_item **new_i) +static struct config_item *make_comm(struct config_group *g, const char *name) { struct comm *cm; cm = kzalloc(sizeof(struct comm), GFP_KERNEL); if (!cm) - return -ENOMEM; + return NULL; config_item_init_type_name(&cm->item, name, &comm_type); cm->nodeid = -1; cm->local = 0; cm->addr_count = 0; - *new_i = &cm->item; - return 0; + return &cm->item; } static void drop_comm(struct config_group *g, struct config_item *i) @@ -563,15 +554,14 @@ static void release_comm(struct config_item *i) kfree(cm); } -static int make_node(struct config_group *g, const char *name, - struct config_item **new_i) +static struct config_item *make_node(struct config_group *g, const char *name) { struct space *sp = to_space(g->cg_item.ci_parent); struct node *nd; nd = kzalloc(sizeof(struct node), GFP_KERNEL); if (!nd) - return -ENOMEM; + return NULL; config_item_init_type_name(&nd->item, name, &node_type); nd->nodeid = -1; @@ -583,8 +573,7 @@ static int make_node(struct config_group *g, const char *name, sp->members_count++; mutex_unlock(&sp->members_lock); - *new_i = &nd->item; - return 0; + return &nd->item; } static void drop_node(struct config_group *g, struct config_item *i) diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c index 443d108211ab..f02ccb34604d 100644 --- a/fs/ocfs2/cluster/heartbeat.c +++ b/fs/ocfs2/cluster/heartbeat.c @@ -1489,28 +1489,25 @@ static struct o2hb_heartbeat_group *to_o2hb_heartbeat_group(struct config_group : NULL; } -static int o2hb_heartbeat_group_make_item(struct config_group *group, - const char *name, - struct config_item **new_item) +static struct config_item *o2hb_heartbeat_group_make_item(struct config_group *group, + const char *name) { struct o2hb_region *reg = NULL; - int ret = 0; + struct config_item *ret = NULL; reg = kzalloc(sizeof(struct o2hb_region), GFP_KERNEL); - if (reg == NULL) { - ret = -ENOMEM; - goto out; - } + if (reg == NULL) + goto out; /* ENOMEM */ config_item_init_type_name(®->hr_item, name, &o2hb_region_type); - *new_item = ®->hr_item; + ret = ®->hr_item; spin_lock(&o2hb_live_lock); list_add_tail(®->hr_all_item, &o2hb_all_regions); spin_unlock(&o2hb_live_lock); out: - if (ret) + if (ret == NULL) kfree(reg); return ret; diff --git a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c index b364b7052e46..cfdb08b484ed 100644 --- a/fs/ocfs2/cluster/nodemanager.c +++ b/fs/ocfs2/cluster/nodemanager.c @@ -644,32 +644,27 @@ out: return ret; } -static int o2nm_node_group_make_item(struct config_group *group, - const char *name, - struct config_item **new_item) +static struct config_item *o2nm_node_group_make_item(struct config_group *group, + const char *name) { struct o2nm_node *node = NULL; - int ret = 0; + struct config_item *ret = NULL; - if (strlen(name) > O2NM_MAX_NAME_LEN) { - ret = -ENAMETOOLONG; - goto out; - } + if (strlen(name) > O2NM_MAX_NAME_LEN) + goto out; /* ENAMETOOLONG */ node = kzalloc(sizeof(struct o2nm_node), GFP_KERNEL); - if (node == NULL) { - ret = -ENOMEM; - goto out; - } + if (node == NULL) + goto out; /* ENOMEM */ strcpy(node->nd_name, name); /* use item.ci_namebuf instead? */ config_item_init_type_name(&node->nd_item, name, &o2nm_node_type); spin_lock_init(&node->nd_lock); - *new_item = &node->nd_item; + ret = &node->nd_item; out: - if (ret) + if (ret == NULL) kfree(node); return ret; @@ -756,31 +751,25 @@ static struct o2nm_cluster_group *to_o2nm_cluster_group(struct config_group *gro } #endif -static int o2nm_cluster_group_make_group(struct config_group *group, - const char *name, - struct config_group **new_group) +static struct config_group *o2nm_cluster_group_make_group(struct config_group *group, + const char *name) { struct o2nm_cluster *cluster = NULL; struct o2nm_node_group *ns = NULL; - struct config_group *o2hb_group = NULL; + struct config_group *o2hb_group = NULL, *ret = NULL; void *defs = NULL; - int ret = 0; /* this runs under the parent dir's i_mutex; there can be only * one caller in here at a time */ - if (o2nm_single_cluster) { - ret = -ENOSPC; - goto out; - } + if (o2nm_single_cluster) + goto out; /* ENOSPC */ cluster = kzalloc(sizeof(struct o2nm_cluster), GFP_KERNEL); ns = kzalloc(sizeof(struct o2nm_node_group), GFP_KERNEL); defs = kcalloc(3, sizeof(struct config_group *), GFP_KERNEL); o2hb_group = o2hb_alloc_hb_set(); - if (cluster == NULL || ns == NULL || o2hb_group == NULL || defs == NULL) { - ret = -ENOMEM; + if (cluster == NULL || ns == NULL || o2hb_group == NULL || defs == NULL) goto out; - } config_group_init_type_name(&cluster->cl_group, name, &o2nm_cluster_type); @@ -797,11 +786,11 @@ static int o2nm_cluster_group_make_group(struct config_group *group, cluster->cl_idle_timeout_ms = O2NET_IDLE_TIMEOUT_MS_DEFAULT; cluster->cl_keepalive_delay_ms = O2NET_KEEPALIVE_DELAY_MS_DEFAULT; - *new_group = &cluster->cl_group; + ret = &cluster->cl_group; o2nm_single_cluster = cluster; out: - if (ret) { + if (ret == NULL) { kfree(cluster); kfree(ns); o2hb_free_hb_set(o2hb_group); diff --git a/include/linux/configfs.h b/include/linux/configfs.h index 0488f937634a..3ae65b1bf90f 100644 --- a/include/linux/configfs.h +++ b/include/linux/configfs.h @@ -165,8 +165,8 @@ struct configfs_item_operations { }; struct configfs_group_operations { - int (*make_item)(struct config_group *group, const char *name, struct config_item **new_item); - int (*make_group)(struct config_group *group, const char *name, struct config_group **new_group); + struct config_item *(*make_item)(struct config_group *group, const char *name); + struct config_group *(*make_group)(struct config_group *group, const char *name); int (*commit_item)(struct config_item *item); void (*disconnect_notify)(struct config_group *group, struct config_item *item); void (*drop_item)(struct config_group *group, struct config_item *item); -- cgit v1.2.2 From a6795e9ebb420d87af43789174689af0d66d1d35 Mon Sep 17 00:00:00 2001 From: Joel Becker Date: Thu, 17 Jul 2008 15:21:29 -0700 Subject: configfs: Allow ->make_item() and ->make_group() to return detailed errors. The configfs operations ->make_item() and ->make_group() currently return a new item/group. A return of NULL signifies an error. Because of this, -ENOMEM is the only return code bubbled up the stack. Multiple folks have requested the ability to return specific error codes when these operations fail. This patch adds that ability by changing the ->make_item/group() ops to return ERR_PTR() values. These errors are bubbled up appropriately. NULL returns are changed to -ENOMEM for compatibility. Also updated are the in-kernel users of configfs. This is a rework of reverted commit 11c3b79218390a139f2d474ee1e983a672d5839a. Signed-off-by: Joel Becker --- .../filesystems/configfs/configfs_example.c | 4 ++-- drivers/net/netconsole.c | 2 +- fs/configfs/dir.c | 25 +++++++++++++--------- fs/dlm/config.c | 8 +++---- fs/ocfs2/cluster/heartbeat.c | 10 ++------- fs/ocfs2/cluster/nodemanager.c | 16 +++++--------- include/linux/configfs.h | 3 ++- 7 files changed, 31 insertions(+), 37 deletions(-) diff --git a/Documentation/filesystems/configfs/configfs_example.c b/Documentation/filesystems/configfs/configfs_example.c index 25151fd5c2c6..039648791701 100644 --- a/Documentation/filesystems/configfs/configfs_example.c +++ b/Documentation/filesystems/configfs/configfs_example.c @@ -279,7 +279,7 @@ static struct config_item *simple_children_make_item(struct config_group *group, simple_child = kzalloc(sizeof(struct simple_child), GFP_KERNEL); if (!simple_child) - return NULL; + return ERR_PTR(-ENOMEM); config_item_init_type_name(&simple_child->item, name, @@ -366,7 +366,7 @@ static struct config_group *group_children_make_group(struct config_group *group simple_children = kzalloc(sizeof(struct simple_children), GFP_KERNEL); if (!simple_children) - return NULL; + return ERR_PTR(-ENOMEM); config_group_init_type_name(&simple_children->group, name, diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 665341e43055..e13966bb5f77 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -598,7 +598,7 @@ static struct config_item *make_netconsole_target(struct config_group *group, nt = kzalloc(sizeof(*nt), GFP_KERNEL); if (!nt) { printk(KERN_ERR "netconsole: failed to allocate memory\n"); - return NULL; + return ERR_PTR(-ENOMEM); } nt->np.name = "netconsole"; diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 614e382a6049..179589be063a 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -1027,9 +1027,10 @@ EXPORT_SYMBOL(configfs_undepend_item); static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) { - int ret, module_got = 0; - struct config_group *group; - struct config_item *item; + int ret = 0; + int module_got = 0; + struct config_group *group = NULL; + struct config_item *item = NULL; struct config_item *parent_item; struct configfs_subsystem *subsys; struct configfs_dirent *sd; @@ -1070,28 +1071,32 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) snprintf(name, dentry->d_name.len + 1, "%s", dentry->d_name.name); mutex_lock(&subsys->su_mutex); - group = NULL; - item = NULL; if (type->ct_group_ops->make_group) { group = type->ct_group_ops->make_group(to_config_group(parent_item), name); - if (group) { + if (!group) + group = ERR_PTR(-ENOMEM); + if (!IS_ERR(group)) { link_group(to_config_group(parent_item), group); item = &group->cg_item; - } + } else + ret = PTR_ERR(group); } else { item = type->ct_group_ops->make_item(to_config_group(parent_item), name); - if (item) + if (!item) + item = ERR_PTR(-ENOMEM); + if (!IS_ERR(item)) link_obj(parent_item, item); + else + ret = PTR_ERR(item); } mutex_unlock(&subsys->su_mutex); kfree(name); - if (!item) { + if (ret) { /* * If item == NULL, then link_obj() was never called. * There are no extra references to clean up. */ - ret = -ENOMEM; goto out_put; } diff --git a/fs/dlm/config.c b/fs/dlm/config.c index eac23bd288b2..c4e7d721bd8d 100644 --- a/fs/dlm/config.c +++ b/fs/dlm/config.c @@ -438,7 +438,7 @@ static struct config_group *make_cluster(struct config_group *g, kfree(gps); kfree(sps); kfree(cms); - return NULL; + return ERR_PTR(-ENOMEM); } static void drop_cluster(struct config_group *g, struct config_item *i) @@ -495,7 +495,7 @@ static struct config_group *make_space(struct config_group *g, const char *name) kfree(sp); kfree(gps); kfree(nds); - return NULL; + return ERR_PTR(-ENOMEM); } static void drop_space(struct config_group *g, struct config_item *i) @@ -528,7 +528,7 @@ static struct config_item *make_comm(struct config_group *g, const char *name) cm = kzalloc(sizeof(struct comm), GFP_KERNEL); if (!cm) - return NULL; + return ERR_PTR(-ENOMEM); config_item_init_type_name(&cm->item, name, &comm_type); cm->nodeid = -1; @@ -561,7 +561,7 @@ static struct config_item *make_node(struct config_group *g, const char *name) nd = kzalloc(sizeof(struct node), GFP_KERNEL); if (!nd) - return NULL; + return ERR_PTR(-ENOMEM); config_item_init_type_name(&nd->item, name, &node_type); nd->nodeid = -1; diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c index f02ccb34604d..7dce1612553e 100644 --- a/fs/ocfs2/cluster/heartbeat.c +++ b/fs/ocfs2/cluster/heartbeat.c @@ -1493,24 +1493,18 @@ static struct config_item *o2hb_heartbeat_group_make_item(struct config_group *g const char *name) { struct o2hb_region *reg = NULL; - struct config_item *ret = NULL; reg = kzalloc(sizeof(struct o2hb_region), GFP_KERNEL); if (reg == NULL) - goto out; /* ENOMEM */ + return ERR_PTR(-ENOMEM); config_item_init_type_name(®->hr_item, name, &o2hb_region_type); - ret = ®->hr_item; - spin_lock(&o2hb_live_lock); list_add_tail(®->hr_all_item, &o2hb_all_regions); spin_unlock(&o2hb_live_lock); -out: - if (ret == NULL) - kfree(reg); - return ret; + return ®->hr_item; } static void o2hb_heartbeat_group_drop_item(struct config_group *group, diff --git a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c index cfdb08b484ed..816a3f61330c 100644 --- a/fs/ocfs2/cluster/nodemanager.c +++ b/fs/ocfs2/cluster/nodemanager.c @@ -648,26 +648,19 @@ static struct config_item *o2nm_node_group_make_item(struct config_group *group, const char *name) { struct o2nm_node *node = NULL; - struct config_item *ret = NULL; if (strlen(name) > O2NM_MAX_NAME_LEN) - goto out; /* ENAMETOOLONG */ + return ERR_PTR(-ENAMETOOLONG); node = kzalloc(sizeof(struct o2nm_node), GFP_KERNEL); if (node == NULL) - goto out; /* ENOMEM */ + return ERR_PTR(-ENOMEM); strcpy(node->nd_name, name); /* use item.ci_namebuf instead? */ config_item_init_type_name(&node->nd_item, name, &o2nm_node_type); spin_lock_init(&node->nd_lock); - ret = &node->nd_item; - -out: - if (ret == NULL) - kfree(node); - - return ret; + return &node->nd_item; } static void o2nm_node_group_drop_item(struct config_group *group, @@ -762,7 +755,7 @@ static struct config_group *o2nm_cluster_group_make_group(struct config_group *g /* this runs under the parent dir's i_mutex; there can be only * one caller in here at a time */ if (o2nm_single_cluster) - goto out; /* ENOSPC */ + return ERR_PTR(-ENOSPC); cluster = kzalloc(sizeof(struct o2nm_cluster), GFP_KERNEL); ns = kzalloc(sizeof(struct o2nm_node_group), GFP_KERNEL); @@ -795,6 +788,7 @@ out: kfree(ns); o2hb_free_hb_set(o2hb_group); kfree(defs); + ret = ERR_PTR(-ENOMEM); } return ret; diff --git a/include/linux/configfs.h b/include/linux/configfs.h index 3ae65b1bf90f..d62c19ff041c 100644 --- a/include/linux/configfs.h +++ b/include/linux/configfs.h @@ -148,7 +148,8 @@ struct configfs_attribute { * items. If the item is a group, it may support mkdir(2). * Groups supply one of make_group() and make_item(). If the * group supports make_group(), one can create group children. If it - * supports make_item(), one can create config_item children. If it has + * supports make_item(), one can create config_item children. make_group() + * and make_item() return ERR_PTR() on errors. If it has * default_groups on group->default_groups, it has automatically created * group children. default_groups may coexist alongsize make_group() or * make_item(), but if the group wishes to have only default_groups -- cgit v1.2.2