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 <john.hammond@intel.com>
Change-Id: I8d452781afe4b7c174c3b5fac9f4f0e573a3d85c
Reviewed-on: http://review.whamcloud.com/5161
Tested-by: Hudson
Reviewed-by: Emoly Liu <emoly.liu@intel.com>
Reviewed-by: Keith Mannthey <keith.mannthey@intel.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>

diff --git a/lustre/ldlm/ldlm_pool.c b/lustre/ldlm/ldlm_pool.c
index d32ec5f..8782652 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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;
 }