if client_disconnect_export was called without force flag set,

and exist connect request in flight, this can produce access to
NULL pointer (or already free pointer) when connect_interpret
store ocd flags in obd_namespace.

b=14260
i=adilger
i=johann

diff --git a/lustre/ChangeLog b/lustre/ChangeLog
index 5adb04d..638001a 100644 (file)
--- a/lustre/ChangeLog
+++ b/lustre/ChangeLog
@@ -12,6 +12,15 @@
        * Recommended e2fsprogs version: 1.40.2-cfs4
        * Note that reiserfs quotas are disabled on SLES 10 in this kernel.
 
+Severity   : major
+Bugzilla   : 14260
+Frequency  : rare, at shutdown
+Description: access already free / zero obd_namespace.
+Details    : if client_disconnect_export was called without force flag set,
+             and exist connect request in flight, this can produce access to
+            NULL pointer (or already free pointer) when connect_interpret 
+            store ocd flags in obd_namespace.
+
 Severity   : minor
 Bugzilla   : 14418
 Frequency  : only at startup

diff --git a/lustre/ldlm/ldlm_internal.h b/lustre/ldlm/ldlm_internal.h
index 8055580..ebfbaac 100644 (file)
--- a/lustre/ldlm/ldlm_internal.h
+++ b/lustre/ldlm/ldlm_internal.h
@@ -52,7 +52,8 @@ int ldlm_cancel_lru_local(struct ldlm_namespace *ns, struct list_head *cancels,
 int ldlm_resource_putref_locked(struct ldlm_resource *res);
 void ldlm_resource_insert_lock_after(struct ldlm_lock *original,
                                      struct ldlm_lock *new);
-
+int ldlm_namespace_free_prior(struct ldlm_namespace *ns);
+int ldlm_namespace_free_post(struct ldlm_namespace *ns, int force);
 /* ldlm_lock.c */
 
 /* Number of blocking/completion callbacks that will be sent in

diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c
index abf719f..3303ab5 100644 (file)
--- a/lustre/ldlm/ldlm_lib.c
+++ b/lustre/ldlm/ldlm_lib.c
@@ -37,6 +37,8 @@
 #include <lustre_dlm.h>
 #include <lustre_net.h>
 #include <lustre_sec.h>
+#include "ldlm_internal.h"
+
 
 /* @priority: if non-zero, move the selected to the list head
  * @create: if zero, only search in existed connections
@@ -369,6 +371,7 @@ int client_connect_import(const struct lu_env *env,
         struct obd_import *imp = cli->cl_import;
         struct obd_export *exp;
         struct obd_connect_data *ocd;
+        struct ldlm_namespace *to_be_freed = NULL;
         int rc;
         ENTRY;
 
@@ -425,7 +428,8 @@ int client_connect_import(const struct lu_env *env,
 
         if (rc) {
 out_ldlm:
-                ldlm_namespace_free(obd->obd_namespace, 0);
+                ldlm_namespace_free_prior(obd->obd_namespace);
+                to_be_freed = obd->obd_namespace;
                 obd->obd_namespace = NULL;
 out_disco:
                 cli->cl_conn_count--;
@@ -435,6 +439,9 @@ out_disco:
         }
 out_sem:
         mutex_up(&cli->cl_sem);
+        if (to_be_freed)
+                ldlm_namespace_free_post(to_be_freed, 1);
+
         return rc;
 }
 
@@ -444,6 +451,7 @@ int client_disconnect_export(struct obd_export *exp)
         struct client_obd *cli;
         struct obd_import *imp;
         int rc = 0, err;
+        struct ldlm_namespace *to_be_freed = NULL;
         ENTRY;
 
         if (!obd) {
@@ -483,14 +491,18 @@ int client_disconnect_export(struct obd_export *exp)
                 ldlm_cli_cancel_unused(obd->obd_namespace, NULL,
                                        obd->obd_force ? LDLM_FL_LOCAL_ONLY:0,
                                        NULL);
-                ldlm_namespace_free(obd->obd_namespace, obd->obd_force);
-                obd->obd_namespace = NULL;
+                ldlm_namespace_free_prior(obd->obd_namespace);
+                to_be_freed = obd->obd_namespace;
         }
 
-        if (!obd->obd_force)
-                rc = ptlrpc_disconnect_import(imp, 0);
+        rc = ptlrpc_disconnect_import(imp, 0);
 
         ptlrpc_invalidate_import(imp);
+        /* set obd_namespace to NULL only after invalidate, because we can have
+         * some connect requests in flight, and his need store a connect flags
+         * in obd_namespace. bug 14260 */
+        obd->obd_namespace = NULL;
+
         ptlrpc_free_rq_pool(imp->imp_rq_pool);
         destroy_import(imp);
         cli->cl_import = NULL;
@@ -502,6 +514,9 @@ int client_disconnect_export(struct obd_export *exp)
                 rc = err;
  out_sem:
         mutex_up(&cli->cl_sem);
+        if (to_be_freed)
+                ldlm_namespace_free_post(to_be_freed, obd->obd_force);
+
         RETURN(rc);
 }
 

diff --git a/lustre/ldlm/ldlm_resource.c b/lustre/ldlm/ldlm_resource.c
index 16d831f..2f74265 100644 (file)
--- a/lustre/ldlm/ldlm_resource.c
+++ b/lustre/ldlm/ldlm_resource.c
@@ -472,7 +472,7 @@ int ldlm_namespace_cleanup(struct ldlm_namespace *ns, int flags)
 }
 
 /* Cleanup, but also free, the namespace */
-int ldlm_namespace_free(struct ldlm_namespace *ns, int force)
+int ldlm_namespace_free_prior(struct ldlm_namespace *ns)
 {
         ENTRY;
         if (!ns)
@@ -487,19 +487,6 @@ int ldlm_namespace_free(struct ldlm_namespace *ns, int force)
         /* At shutdown time, don't call the cancellation callback */
         ldlm_namespace_cleanup(ns, 0);
 
-#ifdef LPROCFS
-        {
-                struct proc_dir_entry *dir;
-                dir = lprocfs_srch(ldlm_ns_proc_dir, ns->ns_name);
-                if (dir == NULL) {
-                        CERROR("dlm namespace %s has no procfs dir?\n",
-                               ns->ns_name);
-                } else {
-                        lprocfs_remove(&dir);
-                }
-        }
-#endif
-
         if (ns->ns_refcount > 0) {
                 struct l_wait_info lwi = LWI_INTR(LWI_ON_SIGNAL_NOOP, NULL);
                 int rc;
@@ -520,15 +507,60 @@ int ldlm_namespace_free(struct ldlm_namespace *ns, int force)
                        "dlm namespace %s free done waiting\n", ns->ns_name);
         }
 
+        RETURN(ELDLM_OK);
+}
+
+int ldlm_namespace_free_post(struct ldlm_namespace *ns, int force)
+{
+        ENTRY;
+        if (!ns)
+                RETURN(ELDLM_OK);
+
+#ifdef LPROCFS
+        {
+                struct proc_dir_entry *dir;
+                dir = lprocfs_srch(ldlm_ns_proc_dir, ns->ns_name);
+                if (dir == NULL) {
+                        CERROR("dlm namespace %s has no procfs dir?\n",
+                               ns->ns_name);
+                } else {
+                        lprocfs_remove(&dir);
+                }
+        }
+#endif
         OBD_VFREE(ns->ns_hash, sizeof(*ns->ns_hash) * RES_HASH_SIZE);
         OBD_FREE(ns->ns_name, strlen(ns->ns_name) + 1);
         OBD_FREE_PTR(ns);
-
         ldlm_put_ref(force);
-
         RETURN(ELDLM_OK);
 }
 
+
+/* Cleanup the resource, and free namespace.
+ * bug 12864:
+ * Deadlock issue:
+ * proc1: destroy import
+ *        class_disconnect_export(grab cl_sem) ->
+ *              -> ldlm_namespace_free ->
+ *              -> lprocfs_remove(grab _lprocfs_lock).
+ * proc2: read proc info
+ *        lprocfs_fops_read(grab _lprocfs_lock) ->
+ *              -> osc_rd_active, etc(grab cl_sem).
+ *
+ * So that I have to split the ldlm_namespace_free into two parts - the first
+ * part ldlm_namespace_free_prior is used to cleanup the resource which is
+ * being used; the 2nd part ldlm_namespace_free_post is used to unregister the
+ * lprocfs entries, and then free memory. It will be called w/o cli->cl_sem
+ * held.
+ */
+int ldlm_namespace_free(struct ldlm_namespace *ns, int force)
+{
+        ldlm_namespace_free_prior(ns);
+        ldlm_namespace_free_post(ns, force);
+        return ELDLM_OK;
+}
+
+
 void ldlm_namespace_get_nolock(struct ldlm_namespace *ns)
 {
         LASSERT(ns->ns_refcount >= 0);

diff --git a/lustre/obdclass/obd_mount.c b/lustre/obdclass/obd_mount.c
index 5c24bf1..a09dc36 100644 (file)
--- a/lustre/obdclass/obd_mount.c
+++ b/lustre/obdclass/obd_mount.c
@@ -770,9 +770,8 @@ static int lustre_stop_mgc(struct super_block *sb)
                 GOTO(out, rc = -EBUSY);
         }
 
-        /* MGC should disconnect nicely so MGS won't print eviction messages */
-        obd->obd_force = (lsi->lsi_flags & LSI_UMOUNT_FORCE) != 0;
-        /* The MGC has no recoverable data in any case. */
+        /* The MGC has no recoverable data in any case. 
+         * force shotdown set in umount_begin */
         obd->obd_no_recov = 1;
 
         if (obd->u.cli.cl_mgc_mgsexp)
@@ -1446,12 +1445,10 @@ static void server_put_super(struct super_block *sb)
                 obd = class_name2obd(lsi->lsi_ldd->ldd_svname);
                 if (obd) {
                         CDEBUG(D_MOUNT, "stopping %s\n", obd->obd_name);
-                        if (lsi->lsi_flags & LSI_UMOUNT_FORCE)
-                                obd->obd_force = 1;
                         if (lsi->lsi_flags & LSI_UMOUNT_FAILOVER)
                                 obd->obd_fail = 1;
                         /* We can't seem to give an error return code
-                           to .put_super, so we better make sure we clean up! */
+                         * to .put_super, so we better make sure we clean up! */
                         obd->obd_force = 1;
                         class_manual_cleanup(obd);
                 } else {

diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c
index 765df31..e85a7f8 100644 (file)
--- a/lustre/ptlrpc/client.c
+++ b/lustre/ptlrpc/client.c
@@ -1026,14 +1026,13 @@ check_ctx:
                 if (req->rq_bulk != NULL)
                         ptlrpc_unregister_bulk (req);
 
-                req->rq_phase = RQ_PHASE_COMPLETE;
-
                 if (req->rq_interpret_reply != NULL) {
                         int (*interpreter)(struct ptlrpc_request *,void *,int) =
                                 req->rq_interpret_reply;
                         req->rq_status = interpreter(req, &req->rq_async_args,
                                                      req->rq_status);
                 }
+                req->rq_phase = RQ_PHASE_COMPLETE;
 
                 CDEBUG(D_RPCTRACE, "Completed RPC pname:cluuid:pid:xid:nid:"
                        "opc %s:%s:%d:"LPU64":%s:%d\n", cfs_curproc_comm(),

diff --git a/lustre/ptlrpc/import.c b/lustre/ptlrpc/import.c
index 2597872..737a86a 100644 (file)
--- a/lustre/ptlrpc/import.c
+++ b/lustre/ptlrpc/import.c
@@ -1021,8 +1021,12 @@ int ptlrpc_disconnect_import(struct obd_import *imp, int noclose)
 {
         struct ptlrpc_request *req;
         int rq_opc, rc = 0;
+        int nowait = imp->imp_obd->obd_force;
         ENTRY;
 
+        if (nowait)
+                GOTO(set_state, rc);
+
         switch (imp->imp_connect_op) {
         case OST_CONNECT: rq_opc = OST_DISCONNECT; break;
         case MDS_CONNECT: rq_opc = MDS_DISCONNECT; break;
@@ -1074,6 +1078,7 @@ int ptlrpc_disconnect_import(struct obd_import *imp, int noclose)
                 ptlrpc_req_finished(req);
         }
 
+set_state:
         spin_lock(&imp->imp_lock);
 out:
         if (noclose)