LU-17269 obdclass: fix locking for class_register/deregister

Prevent registration and deregistration from racing with
each other. Otherwise, we could see a crash.

Test-Parameters: testlist=conf-sanity env=ONLY=41c,ONLY_REPEAT=30
Signed-off-by: Timothy Day <timday@amazon.com>
Change-Id: I4d512dcc8778c5116c1d6037ed2b7f486a7bc0dc
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54747
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c
index 7c15eba..b75aaaa 100644 (file)
--- a/lustre/obdclass/genops.c
+++ b/lustre/obdclass/genops.c
@@ -480,6 +480,45 @@ void class_free_dev(struct obd_device *obd)
        class_put_type(obd_type);
 }
 
+static int class_name2dev_nolock(const char *name)
+{
+       struct obd_device *obd = NULL;
+       unsigned long dev_no = 0;
+       int ret;
+
+       if (!name)
+               return -1;
+
+       obd_device_for_each(dev_no, obd) {
+               if (strcmp(name, obd->obd_name) == 0) {
+                       /*
+                        * Make sure we finished attaching before we give
+                        * out any references
+                        */
+                       LASSERT(obd->obd_magic == OBD_DEVICE_MAGIC);
+                       if (obd->obd_attached) {
+                               ret = obd->obd_minor;
+                               return ret;
+                       }
+                       break;
+               }
+       }
+
+       return -1;
+}
+
+int class_name2dev(const char *name)
+{
+       int ret;
+
+       obd_device_lock();
+       ret = class_name2dev_nolock(name);
+       obd_device_unlock();
+
+       return ret;
+}
+EXPORT_SYMBOL(class_name2dev);
+
 /**
  * Unregister obd device.
  *
@@ -491,12 +530,14 @@ void class_free_dev(struct obd_device *obd)
  */
 void class_unregister_device(struct obd_device *obd)
 {
+       obd_device_lock();
        if (obd->obd_minor >= 0) {
-               xa_erase(&obd_devs, obd->obd_minor);
+               __xa_erase(&obd_devs, obd->obd_minor);
                class_decref(obd, "obd_device_list", obd);
                obd->obd_minor = -1;
                atomic_dec(&obd_devs_count);
        }
+       obd_device_unlock();
 }
 
 /**
@@ -523,10 +564,11 @@ int class_register_device(struct obd_device *new_obd)
        if (class_name2dev(new_obd->obd_name) != -1)
                obd_zombie_barrier();
 
-       if (class_name2dev(new_obd->obd_name) == -1) {
+       obd_device_lock();
+       if (class_name2dev_nolock(new_obd->obd_name) == -1) {
                class_incref(new_obd, "obd_device_list", new_obd);
-               rc = xa_alloc(&obd_devs, &dev_no, new_obd,
-                             xa_limit_31b, GFP_ATOMIC);
+               rc = __xa_alloc(&obd_devs, &dev_no, new_obd,
+                               xa_limit_31b, GFP_ATOMIC);
 
                if (rc != 0)
                        goto out;
@@ -538,39 +580,9 @@ int class_register_device(struct obd_device *new_obd)
        }
 
 out:
-       RETURN(rc);
-}
-
-int class_name2dev(const char *name)
-{
-       struct obd_device *obd = NULL;
-       unsigned long dev_no = 0;
-       int ret;
-
-       if (!name)
-               return -1;
-
-       obd_device_lock();
-       obd_device_for_each(dev_no, obd) {
-               if (strcmp(name, obd->obd_name) == 0) {
-                       /*
-                        * Make sure we finished attaching before we give
-                        * out any references
-                        */
-                       LASSERT(obd->obd_magic == OBD_DEVICE_MAGIC);
-                       if (obd->obd_attached) {
-                               ret = obd->obd_minor;
-                               obd_device_unlock();
-                               return ret;
-                       }
-                       break;
-               }
-       }
        obd_device_unlock();
-
-       return -1;
+       RETURN(rc);
 }
-EXPORT_SYMBOL(class_name2dev);
 
 struct obd_device *class_name2obd(const char *name)
 {

diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh
index 1f1ddca..005edab 100755 (executable)
--- a/lustre/tests/conf-sanity.sh
+++ b/lustre/tests/conf-sanity.sh
@@ -15,7 +15,6 @@ init_logging
 ALWAYS_EXCEPT="$CONF_SANITY_EXCEPT 32newtarball"
 
 always_except LU-11915 110
-always_except LU-17269 41c
 
 if $SHARED_KEY; then
        always_except LU-9795 84 86 103
@@ -3645,12 +3644,8 @@ run_test 41b "mount mds with --nosvc and --nomgs on first mount"
 test_41c() {
        local oss_list=$(comma_list $(osts_nodes))
 
-       [[ "$MDS1_VERSION" -ge $(version_code 2.6.52) ]] ||
-       [[ "$MDS1_VERSION" -ge $(version_code 2.5.26) &&
-          "$MDS1_VERSION" -lt $(version_code 2.5.50) ]] ||
-       [[ "$MDS1_VERSION" -ge $(version_code 2.5.4) &&
-          "$MDS1_VERSION" -lt $(version_code 2.5.11) ]] ||
-               skip "Need MDS version 2.5.4+ or 2.5.26+ or 2.6.52+"
+       (( "$MDS1_VERSION" >= $(version_code 2.15.62.4) )) ||
+               skip "Need MDS >= 2.15.62.4 for parallel device locking"
 
        # ensure mds1 ost1 have been created even if running sub-test standalone
        cleanup
@@ -3671,7 +3666,7 @@ test_41c() {
        local mds1mnt=$(facet_mntpt mds1)
        local mds1opts=$MDS_MOUNT_OPTS
 
-       if [ "$mds1_FSTYPE" == ldiskfs ] &&
+       if [[ "$mds1_FSTYPE" == ldiskfs ]] &&
           ! do_facet mds1 test -b $mds1dev; then
                mds1opts=$(csa_add "$mds1opts" -o loop)
        fi