move ldlm namespace creation in setup phase to avoid grab

_lprocfs_lock with cli_sem held.

Branch HEAD
b=18380
i=rread

diff --git a/lustre/ChangeLog b/lustre/ChangeLog
index 6c55f96..dbfe209 100644 (file)
--- a/lustre/ChangeLog
+++ b/lustre/ChangeLog
@@ -13,6 +13,12 @@ tbd  Sun Microsystems, Inc.
         removed cwd "./" (refer to Bugzilla 14399).
        * File join has been disabled in this release, refer to Bugzilla 16929.
 
         removed cwd "./" (refer to Bugzilla 14399).
        * File join has been disabled in this release, refer to Bugzilla 16929.
 

+Severity   : normal
+Frequency  : rare
+Bugzilla   : 18380
+Descriptoin: lock ordering violation between &cli->cl_sem and _lprocfs_lock
+Details    : move ldlm namespace creation in setup phase to avoid grab
+            _lprocfs_lock with cli_sem held.

 
 Severity   : normal
 Bugzilla   : 19507
 
 Severity   : normal
 Bugzilla   : 19507

diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c
index b4e9d61..f45f7e8 100644 (file)
--- a/lustre/ldlm/ldlm_lib.c
+++ b/lustre/ldlm/ldlm_lib.c
@@ -359,11 +359,20 @@ int client_obd_setup(struct obd_device *obddev, struct lustre_cfg *lcfg)
                                name, obddev->obd_name,
                                cli->cl_target_uuid.uuid);
                         spin_lock(&imp->imp_lock);
                                name, obddev->obd_name,
                                cli->cl_target_uuid.uuid);
                         spin_lock(&imp->imp_lock);

-                        imp->imp_invalid = 1;
+                        imp->imp_deactive = 1;

                         spin_unlock(&imp->imp_lock);
                 }
         }
 
                         spin_unlock(&imp->imp_lock);
                 }
         }
 

+        obddev->obd_namespace = ldlm_namespace_new(obddev, obddev->obd_name,
+                                                   LDLM_NAMESPACE_CLIENT,
+                                                   LDLM_NAMESPACE_GREEDY);
+        if (obddev->obd_namespace == NULL) {
+                CERROR("Unable to create client namespace - %s\n",
+                       obddev->obd_name);
+                GOTO(err_import, rc = -ENOMEM);
+        }
+

         cli->cl_qchk_stat = CL_NOT_QUOTACHECKED;
 
         RETURN(rc);
         cli->cl_qchk_stat = CL_NOT_QUOTACHECKED;
 
         RETURN(rc);


@@ -380,6 +389,10 @@ err:
 int client_obd_cleanup(struct obd_device *obddev)
 {
         ENTRY;
 int client_obd_cleanup(struct obd_device *obddev)
 {
         ENTRY;

+
+        ldlm_namespace_free_post(obddev->obd_namespace);
+        obddev->obd_namespace = NULL;
+

         ldlm_put_ref();
         RETURN(0);
 }
         ldlm_put_ref();
         RETURN(0);
 }


@@ -393,7 +406,6 @@ int client_connect_import(const struct lu_env *env,
         struct client_obd *cli = &obd->u.cli;
         struct obd_import *imp = cli->cl_import;
         struct obd_connect_data *ocd;
         struct client_obd *cli = &obd->u.cli;
         struct obd_import *imp = cli->cl_import;
         struct obd_connect_data *ocd;

-        struct ldlm_namespace *to_be_freed = NULL;

         struct lustre_handle conn = { 0 };
         int rc;
         ENTRY;
         struct lustre_handle conn = { 0 };
         int rc;
         ENTRY;


@@ -406,23 +418,11 @@ int client_connect_import(const struct lu_env *env,
         rc = class_connect(&conn, obd, cluuid);
         if (rc)
                 GOTO(out_sem, rc);
         rc = class_connect(&conn, obd, cluuid);
         if (rc)
                 GOTO(out_sem, rc);

-                
+

         cli->cl_conn_count++;
         *exp = class_conn2export(&conn);
 
         cli->cl_conn_count++;
         *exp = class_conn2export(&conn);
 

-        if (obd->obd_namespace != NULL)
-                CERROR("already have namespace!\n");
-
-        /*
-         * Deadlock case - bug 18380
-         */
-        up_write(&cli->cl_sem);
-        obd->obd_namespace = ldlm_namespace_new(obd, obd->obd_name,
-                                                LDLM_NAMESPACE_CLIENT,
-                                                LDLM_NAMESPACE_GREEDY);
-        down_write(&cli->cl_sem);
-        if (obd->obd_namespace == NULL)
-                GOTO(out_disco, rc = -ENOMEM);
+        LASSERT(obd->obd_namespace);

 
         imp->imp_dlm_handle = conn;
         rc = ptlrpc_init_import(imp);
 
         imp->imp_dlm_handle = conn;
         rc = ptlrpc_init_import(imp);


@@ -455,18 +455,12 @@ int client_connect_import(const struct lu_env *env,
 
         if (rc) {
 out_ldlm:
 
         if (rc) {
 out_ldlm:

-                ldlm_namespace_free_prior(obd->obd_namespace, imp, 0);
-                to_be_freed = obd->obd_namespace;
-                obd->obd_namespace = NULL;
-out_disco:

                 cli->cl_conn_count--;
                 class_disconnect(*exp);
                 *exp = NULL;
         }
 out_sem:
         up_write(&cli->cl_sem);
                 cli->cl_conn_count--;
                 class_disconnect(*exp);
                 *exp = NULL;
         }
 out_sem:
         up_write(&cli->cl_sem);

-        if (to_be_freed)
-                ldlm_namespace_free_post(to_be_freed);

 
         return rc;
 }
 
         return rc;
 }


@@ -477,7 +471,6 @@ int client_disconnect_export(struct obd_export *exp)
         struct client_obd *cli;
         struct obd_import *imp;
         int rc = 0, err;
         struct client_obd *cli;
         struct obd_import *imp;
         int rc = 0, err;

-        struct ldlm_namespace *to_be_freed = NULL;

         ENTRY;
 
         if (!obd) {
         ENTRY;
 
         if (!obd) {


@@ -490,6 +483,9 @@ int client_disconnect_export(struct obd_export *exp)
         imp = cli->cl_import;
 
         down_write(&cli->cl_sem);
         imp = cli->cl_import;
 
         down_write(&cli->cl_sem);

+        CDEBUG(D_INFO, "disconnect %s - %d\n", obd->obd_name,
+               cli->cl_conn_count);
+

         if (!cli->cl_conn_count) {
                 CERROR("disconnecting disconnected device (%s)\n",
                        obd->obd_name);
         if (!cli->cl_conn_count) {
                 CERROR("disconnecting disconnected device (%s)\n",
                        obd->obd_name);


@@ -518,7 +514,6 @@ int client_disconnect_export(struct obd_export *exp)
                                        obd->obd_force ? LDLM_FL_LOCAL_ONLY:0,
                                        NULL);
                 ldlm_namespace_free_prior(obd->obd_namespace, imp, obd->obd_force);
                                        obd->obd_force ? LDLM_FL_LOCAL_ONLY:0,
                                        NULL);
                 ldlm_namespace_free_prior(obd->obd_namespace, imp, obd->obd_force);

-                to_be_freed = obd->obd_namespace;

         }
 
         /*
         }
 
         /*


@@ -530,10 +525,6 @@ int client_disconnect_export(struct obd_export *exp)
         down_write(&cli->cl_sem);
 
         ptlrpc_invalidate_import(imp);
         down_write(&cli->cl_sem);
 
         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;

 
         if (imp->imp_rq_pool) {
                 ptlrpc_free_rq_pool(imp->imp_rq_pool);
 
         if (imp->imp_rq_pool) {
                 ptlrpc_free_rq_pool(imp->imp_rq_pool);


@@ -552,8 +543,6 @@ int client_disconnect_export(struct obd_export *exp)
                 rc = err;
 
         up_write(&cli->cl_sem);
                 rc = err;
 
         up_write(&cli->cl_sem);

-        if (to_be_freed)
-                ldlm_namespace_free_post(to_be_freed);

 
         RETURN(rc);
 }
 
         RETURN(rc);
 }

diff --git a/lustre/ldlm/ldlm_resource.c b/lustre/ldlm/ldlm_resource.c
index e5caa7b..ade6a92 100644 (file)
--- a/lustre/ldlm/ldlm_resource.c
+++ b/lustre/ldlm/ldlm_resource.c
@@ -588,10 +588,6 @@ void ldlm_namespace_free_prior(struct ldlm_namespace *ns,
                 return;
         }
 
                 return;
         }
 

-        /*
-         * Make sure that nobody can find this ns in its list.
-         */
-        ldlm_namespace_unregister(ns, ns->ns_client);

 
         /*
          * Can fail with -EINTR when force == 0 in which case try harder.
 
         /*
          * Can fail with -EINTR when force == 0 in which case try harder.


@@ -626,6 +622,11 @@ void ldlm_namespace_free_post(struct ldlm_namespace *ns)
                 return;
         }
 
                 return;
         }
 

+
+        /*
+         * Make sure that nobody can find this ns in its list.
+         */
+        ldlm_namespace_unregister(ns, ns->ns_client);

         /*
          * Fini pool _before_ parent proc dir is removed. This is important as
          * ldlm_pool_fini() removes own proc dir which is child to @dir. Removing
         /*
          * Fini pool _before_ parent proc dir is removed. This is important as
          * ldlm_pool_fini() removes own proc dir which is child to @dir. Removing