LU-8066 obd_type: discard obd_type_lock

This lock is only used to protect typ_refcnt, so change
that to an atomic_t and discard the lock.

The lock also covers calls to try_module_get and module_put,
but this serves no purpose as it does not prevent the module
from being unloaded.

Finally, the return value for the call to try_module_get is
ignored, which is not safe.

Linux-commit: 493ae16ed39a1c9f792c3b650e2dff11ca2e73e8

Change-Id: I904c51cc4d3426ca520c0bcad9665380ce1f3c3d
Signed-off-by: NeilBrown <neilb@suse.com>
Reviewed-on: https://review.whamcloud.com/35096
Reviewed-by: Shaun Tancheff <stancheff@cray.com>
Reviewed-by: Neil Brown <neilb@suse.de>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/obd.h b/lustre/include/obd.h
index c1c1f37..96c77a0 100644 (file)
--- a/lustre/include/obd.h
+++ b/lustre/include/obd.h
@@ -106,9 +106,8 @@ struct obd_type {
 #ifdef HAVE_SERVER_SUPPORT
        bool                     typ_sym_filter;
 #endif
 #ifdef HAVE_SERVER_SUPPORT
        bool                     typ_sym_filter;
 #endif

-       int                      typ_refcnt;
+       atomic_t                 typ_refcnt;

        struct lu_device_type   *typ_lu;
        struct lu_device_type   *typ_lu;

-       spinlock_t               obd_type_lock;

        struct kobject           typ_kobj;
 };
 #define typ_name typ_kobj.name
        struct kobject           typ_kobj;
 };
 #define typ_name typ_kobj.name

diff --git a/lustre/mgc/mgc_request.c b/lustre/mgc/mgc_request.c
index 456891a..ff0b0c1 100644 (file)
--- a/lustre/mgc/mgc_request.c
+++ b/lustre/mgc/mgc_request.c
@@ -928,7 +928,7 @@ static int mgc_cleanup(struct obd_device *obd)
 
         /* COMPAT_146 - old config logs may have added profiles we don't
            know about */
 
         /* COMPAT_146 - old config logs may have added profiles we don't
            know about */

-        if (obd->obd_type->typ_refcnt <= 1)
+       if (atomic_read(&obd->obd_type->typ_refcnt) <= 1)

                 /* Only for the last mgc */
                 class_del_profiles();
 
                 /* Only for the last mgc */
                 class_del_profiles();
 

diff --git a/lustre/obdclass/cl_object.c b/lustre/obdclass/cl_object.c
index 7d14a6b..ebfc7f2 100644 (file)
--- a/lustre/obdclass/cl_object.c
+++ b/lustre/obdclass/cl_object.c
@@ -48,7 +48,6 @@
 
 #include <linux/list.h>
 #include <libcfs/libcfs.h>
 
 #include <linux/list.h>
 #include <libcfs/libcfs.h>

-/* class_put_type() */

 #include <obd_class.h>
 #include <obd_support.h>
 #include <lustre_fid.h>
 #include <obd_class.h>
 #include <obd_support.h>
 #include <lustre_fid.h>

diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c
index bba84a9..436e5c7 100644 (file)
--- a/lustre/obdclass/genops.c
+++ b/lustre/obdclass/genops.c
@@ -136,15 +136,17 @@ struct obd_type *class_get_type(const char *name)
         }
 #endif
         if (type) {
         }
 #endif
         if (type) {

-               spin_lock(&type->obd_type_lock);
-               type->typ_refcnt++;
-               try_module_get(type->typ_dt_ops->o_owner);
-               spin_unlock(&type->obd_type_lock);
-               /* class_search_type() returned a counted reference,
-                * but we don't need that count any more as
-                * we have one through typ_refcnt.
-                */
-               kobject_put(&type->typ_kobj);
+               if (try_module_get(type->typ_dt_ops->o_owner)) {
+                       atomic_inc(&type->typ_refcnt);
+                       /* class_search_type() returned a counted reference,
+                        * but we don't need that count any more as
+                        * we have one through typ_refcnt.
+                        */
+                       kobject_put(&type->typ_kobj);
+               } else {
+                       kobject_put(&type->typ_kobj);
+                       type = NULL;
+               }

        }
        return type;
 }
        }
        return type;
 }


@@ -152,10 +154,8 @@ struct obd_type *class_get_type(const char *name)
 void class_put_type(struct obd_type *type)
 {
        LASSERT(type);
 void class_put_type(struct obd_type *type)
 {
        LASSERT(type);

-       spin_lock(&type->obd_type_lock);
-       type->typ_refcnt--;

        module_put(type->typ_dt_ops->o_owner);
        module_put(type->typ_dt_ops->o_owner);

-       spin_unlock(&type->obd_type_lock);
+       atomic_dec(&type->typ_refcnt);

 }
 
 static void class_sysfs_release(struct kobject *kobj)
 }
 
 static void class_sysfs_release(struct kobject *kobj)


@@ -276,7 +276,6 @@ dir_exist:
         /* md_ops is optional */
         if (md_ops)
                 *(type->typ_md_ops) = *md_ops;
         /* md_ops is optional */
         if (md_ops)
                 *(type->typ_md_ops) = *md_ops;

-       spin_lock_init(&type->obd_type_lock);

 
 #ifdef HAVE_SERVER_SUPPORT
        if (type->typ_sym_filter) {
 
 #ifdef HAVE_SERVER_SUPPORT
        if (type->typ_sym_filter) {


@@ -339,8 +338,9 @@ int class_unregister_type(const char *name)
                 RETURN(-EINVAL);
         }
 
                 RETURN(-EINVAL);
         }
 

-        if (type->typ_refcnt) {
-                CERROR("type %s has refcount (%d)\n", name, type->typ_refcnt);
+       if (atomic_read(&type->typ_refcnt)) {
+               CERROR("type %s has refcount (%d)\n", name,
+                      atomic_read(&type->typ_refcnt));

                 /* This is a bad situation, let's make the best of it */
                 /* Remove ops, but leave the name for debugging */
                 OBD_FREE_PTR(type->typ_dt_ops);
                 /* This is a bad situation, let's make the best of it */
                 /* Remove ops, but leave the name for debugging */
                 OBD_FREE_PTR(type->typ_dt_ops);

diff --git a/lustre/obdclass/obd_mount_server.c b/lustre/obdclass/obd_mount_server.c
index 5b5cb20..48b5845 100644 (file)
--- a/lustre/obdclass/obd_mount_server.c
+++ b/lustre/obdclass/obd_mount_server.c
@@ -1101,7 +1101,7 @@ static int server_stop_servers(int lsiflags)
        LASSERTF(type, "Server flags %d, obd %s\n", lsiflags,
                 obd ? obd->obd_name : "NULL");
 
        LASSERTF(type, "Server flags %d, obd %s\n", lsiflags,
                 obd ? obd->obd_name : "NULL");
 

-       type_last = (type->typ_refcnt == 1);
+       type_last = (atomic_read(&type->typ_refcnt) == 1);

 
        class_put_type(type);
        if (obd != NULL && type_last) {
 
        class_put_type(type);
        if (obd != NULL && type_last) {


@@ -1367,6 +1367,10 @@ static int server_start_targets(struct super_block *sb)
        /* hold a type reference and put it at server_stop_servers */
        type = class_get_type(IS_MDT(lsi) ?
                              LUSTRE_MDT_NAME : LUSTRE_OST_NAME);
        /* hold a type reference and put it at server_stop_servers */
        type = class_get_type(IS_MDT(lsi) ?
                              LUSTRE_MDT_NAME : LUSTRE_OST_NAME);

+       if (!type) {
+               mutex_unlock(&server_start_lock);
+               GOTO(out_stop_service, rc = -ENODEV);
+       }

        lsi->lsi_server_started = 1;
        mutex_unlock(&server_start_lock);
        if (OBD_FAIL_PRECHECK(OBD_FAIL_OBD_STOP_MDS_RACE) &&
        lsi->lsi_server_started = 1;
        mutex_unlock(&server_start_lock);
        if (OBD_FAIL_PRECHECK(OBD_FAIL_OBD_STOP_MDS_RACE) &&