From 6f827e863f99f21fc03c6fa55fe7fd4a74966c7e Mon Sep 17 00:00:00 2001 From: "John L. Hammond" Date: Thu, 24 Jan 2013 16:54:36 -0600 Subject: [PATCH] LU-2650 procfs: return -ENOMEM from lprocfs_register() In lprocfs_register(), if proc_mkdir() fails then return ERR_PTR(-ENOMEM) rather than NULL and hold _lprocfs_mutex for the whole function. In lprocfs_remove_nolock() return early if the entry is an error pointer. Improve error handling around lprocfs_register() in a few spots. Signed-off-by: John L. Hammond Change-Id: I8d452781afe4b7c174c3b5fac9f4f0e573a3d85c Reviewed-on: http://review.whamcloud.com/5161 Tested-by: Hudson Reviewed-by: Emoly Liu Reviewed-by: Keith Mannthey Tested-by: Maloo Reviewed-by: Andreas Dilger --- lustre/ldlm/ldlm_pool.c | 12 +++-- lustre/obdclass/lprocfs_status.c | 112 +++++++++++++++++++++++---------------- lustre/ptlrpc/gss/lproc_gss.c | 45 ++++++++-------- 3 files changed, 97 insertions(+), 72 deletions(-) diff --git a/lustre/ldlm/ldlm_pool.c b/lustre/ldlm/ldlm_pool.c index d32ec5f..8782652 100644 --- a/lustre/ldlm/ldlm_pool.c +++ b/lustre/ldlm/ldlm_pool.c @@ -743,11 +743,13 @@ static int ldlm_pool_proc_init(struct ldlm_pool *pl) } pl->pl_proc_dir = lprocfs_register("pool", parent_ns_proc, NULL, NULL); - if (IS_ERR(pl->pl_proc_dir)) { - CERROR("LProcFS failed in ldlm-pool-init\n"); - rc = PTR_ERR(pl->pl_proc_dir); - GOTO(out_free_name, rc); - } + if (IS_ERR(pl->pl_proc_dir)) { + rc = PTR_ERR(pl->pl_proc_dir); + pl->pl_proc_dir = NULL; + CERROR("%s: cannot create 'pool' proc entry: rc = %d\n", + ldlm_ns_name(ns), rc); + GOTO(out_free_name, rc); + } var_name[MAX_STRING_SIZE] = '\0'; memset(pool_vars, 0, sizeof(pool_vars)); diff --git a/lustre/obdclass/lprocfs_status.c b/lustre/obdclass/lprocfs_status.c index 1579559..8b52d52 100644 --- a/lustre/obdclass/lprocfs_status.c +++ b/lustre/obdclass/lprocfs_status.c @@ -294,26 +294,15 @@ struct file_operations lprocfs_evict_client_fops = { }; EXPORT_SYMBOL(lprocfs_evict_client_fops); -/** - * Add /proc entries. - * - * \param root [in] The parent proc entry on which new entry will be added. - * \param list [in] Array of proc entries to be added. - * \param data [in] The argument to be passed when entries read/write routines - * are called through /proc file. - * - * \retval 0 on success - * < 0 on error - */ -int lprocfs_add_vars(struct proc_dir_entry *root, struct lprocfs_vars *list, - void *data) +static int __lprocfs_add_vars(struct proc_dir_entry *root, + struct lprocfs_vars *list, + void *data) { int rc = 0; if (root == NULL || list == NULL) return -EINVAL; - LPROCFS_WRITE_ENTRY(); while (list->name != NULL) { struct proc_dir_entry *cur_root, *proc; char *pathcopy, *cur, *next, pathbuf[64]; @@ -378,21 +367,43 @@ int lprocfs_add_vars(struct proc_dir_entry *root, struct lprocfs_vars *list, list++; } out: - LPROCFS_WRITE_EXIT(); return rc; } + +/** + * Add /proc entries. + * + * \param root [in] The parent proc entry on which new entry will be added. + * \param list [in] Array of proc entries to be added. + * \param data [in] The argument to be passed when entries read/write routines + * are called through /proc file. + * + * \retval 0 on success + * < 0 on error + */ +int lprocfs_add_vars(struct proc_dir_entry *root, struct lprocfs_vars *list, + void *data) +{ + int rc; + + LPROCFS_WRITE_ENTRY(); + rc = __lprocfs_add_vars(root, list, data); + LPROCFS_WRITE_EXIT(); + + return rc; +} EXPORT_SYMBOL(lprocfs_add_vars); -void lprocfs_remove_nolock(struct proc_dir_entry **rooth) +void lprocfs_remove_nolock(struct proc_dir_entry **proot) { - struct proc_dir_entry *root = *rooth; - struct proc_dir_entry *temp = root; - struct proc_dir_entry *rm_entry; - struct proc_dir_entry *parent; + struct proc_dir_entry *root = *proot; + struct proc_dir_entry *temp = root; + struct proc_dir_entry *rm_entry; + struct proc_dir_entry *parent; - if (!root) - return; - *rooth = NULL; + *proot = NULL; + if (root == NULL || IS_ERR(root)) + return; parent = root->parent; LASSERT(parent != NULL); @@ -478,27 +489,34 @@ void lprocfs_try_remove_proc_entry(const char *name, EXPORT_SYMBOL(lprocfs_try_remove_proc_entry); struct proc_dir_entry *lprocfs_register(const char *name, - struct proc_dir_entry *parent, - struct lprocfs_vars *list, void *data) + struct proc_dir_entry *parent, + struct lprocfs_vars *list, void *data) { - struct proc_dir_entry *newchild; + struct proc_dir_entry *entry; + int rc; - newchild = lprocfs_srch(parent, name); - if (newchild != NULL) { - CERROR(" Lproc: Attempting to register %s more than once \n", - name); - return ERR_PTR(-EALREADY); - } + LPROCFS_WRITE_ENTRY(); + entry = __lprocfs_srch(parent, name); + if (entry != NULL) { + CERROR("entry '%s' already registered\n", name); + GOTO(out, entry = ERR_PTR(-EALREADY)); + } - newchild = proc_mkdir(name, parent); - if (newchild != NULL && list != NULL) { - int rc = lprocfs_add_vars(newchild, list, data); - if (rc) { - lprocfs_remove(&newchild); - return ERR_PTR(rc); - } - } - return newchild; + entry = proc_mkdir(name, parent); + if (entry == NULL) + GOTO(out, entry = ERR_PTR(-ENOMEM)); + + if (list != NULL) { + rc = __lprocfs_add_vars(entry, list, data); + if (rc != 0) { + lprocfs_remove_nolock(&entry); + GOTO(out, entry = ERR_PTR(rc)); + } + } +out: + LPROCFS_WRITE_EXIT(); + + return entry; } EXPORT_SYMBOL(lprocfs_register); @@ -1983,11 +2001,13 @@ int lprocfs_exp_setup(struct obd_export *exp, lnet_nid_t *nid, int *newnid) NULL, NULL); OBD_FREE(buffer, LNET_NIDSTR_SIZE); - if (new_stat->nid_proc == NULL) { - CERROR("Error making export directory for nid %s\n", - libcfs_nid2str(*nid)); - GOTO(destroy_new_ns, rc = -ENOMEM); - } + if (IS_ERR(new_stat->nid_proc)) { + rc = PTR_ERR(new_stat->nid_proc); + new_stat->nid_proc = NULL; + CERROR("%s: cannot create proc entry for export %s: rc = %d\n", + obd->obd_name, libcfs_nid2str(*nid), rc); + GOTO(destroy_new_ns, rc); + } entry = lprocfs_add_simple(new_stat->nid_proc, "uuid", lprocfs_exp_rd_uuid, NULL, new_stat, NULL); diff --git a/lustre/ptlrpc/gss/lproc_gss.c b/lustre/ptlrpc/gss/lproc_gss.c index ae198e2..3c5d4ab 100644 --- a/lustre/ptlrpc/gss/lproc_gss.c +++ b/lustre/ptlrpc/gss/lproc_gss.c @@ -196,28 +196,31 @@ void gss_exit_lproc(void) int gss_init_lproc(void) { - int rc; + int rc; spin_lock_init(&gss_stat_oos.oos_lock); - gss_proc_root = lprocfs_register("gss", sptlrpc_proc_root, - gss_lprocfs_vars, NULL); - if (IS_ERR(gss_proc_root)) { - gss_proc_root = NULL; - GOTO(err_out, rc = PTR_ERR(gss_proc_root)); - } - - gss_proc_lk = lprocfs_register("lgss_keyring", gss_proc_root, - gss_lk_lprocfs_vars, NULL); - if (IS_ERR(gss_proc_lk)) { - gss_proc_lk = NULL; - GOTO(err_out, rc = PTR_ERR(gss_proc_root)); - } - - return 0; - -err_out: - CERROR("failed to initialize gss lproc entries: %d\n", rc); - gss_exit_lproc(); - return rc; + gss_proc_root = lprocfs_register("gss", sptlrpc_proc_root, + gss_lprocfs_vars, NULL); + if (IS_ERR(gss_proc_root)) { + rc = PTR_ERR(gss_proc_root); + gss_proc_root = NULL; + GOTO(out, rc); + } + + gss_proc_lk = lprocfs_register("lgss_keyring", gss_proc_root, + gss_lk_lprocfs_vars, NULL); + if (IS_ERR(gss_proc_lk)) { + rc = PTR_ERR(gss_proc_lk); + gss_proc_lk = NULL; + GOTO(out, rc); + } + + return 0; + +out: + CERROR("failed to initialize gss lproc entries: %d\n", rc); + gss_exit_lproc(); + + return rc; } -- 1.8.3.1