Whamcloud - gitweb
LU-11089 obd: remove lock from key register/degister 68/33668/7
authorNeilBrown <neilb@suse.com>
Tue, 21 May 2019 15:07:47 +0000 (11:07 -0400)
committerOleg Drokin <green@whamcloud.com>
Sat, 1 Jun 2019 03:57:53 +0000 (03:57 +0000)
lu_context_key_register() doesn't really need locking.
It can use cmpxchg() to atomically install a key, and
check the result to see if it succeeded.
This requires the key to be completely ready before we
try to install it, so lct_used and lct_reference are
set up first, then reverted on failure.

With this done, lu_context_key_degister() no longer
needs locking. It just need to set the slot to NULL.
This is done with suitable memory barriers so that the
slot cannot be reused until we are completely finished
with it.

Note that I added a warning if the slot holds NULL.
The code currently tested that code, but I don't think it
can really happen.

Linux-commit: f4f30a8fc2c9568b87920e89fe4230530c26148f

Change-Id: I8e81c4694e8df2a2805e0b3104a83aa490c536ec
Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-on: https://review.whamcloud.com/33668
Reviewed-by: Gu Zheng <gzheng@ddn.com>
Tested-by: Jenkins
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/obdclass/lu_object.c

index 3ddade9..908513b 100644 (file)
@@ -1370,19 +1370,23 @@ int lu_context_key_register(struct lu_context_key *key)
         LASSERT(key->lct_owner != NULL);
 
         result = -ENFILE;
         LASSERT(key->lct_owner != NULL);
 
         result = -ENFILE;
-       write_lock(&lu_keys_guard);
+       atomic_set(&key->lct_used, 1);
+       lu_ref_init(&key->lct_reference);
         for (i = 0; i < ARRAY_SIZE(lu_keys); ++i) {
         for (i = 0; i < ARRAY_SIZE(lu_keys); ++i) {
-                if (lu_keys[i] == NULL) {
-                        key->lct_index = i;
-                       atomic_set(&key->lct_used, 1);
-                        lu_keys[i] = key;
-                        lu_ref_init(&key->lct_reference);
-                        result = 0;
-                       atomic_inc(&key_set_version);
-                        break;
-                }
+               if (lu_keys[i])
+                       continue;
+               key->lct_index = i;
+               if (cmpxchg(&lu_keys[i], NULL, key) != NULL)
+                       continue;
+
+               result = 0;
+               atomic_inc(&key_set_version);
+               break;
         }
         }
-       write_unlock(&lu_keys_guard);
+       if (result) {
+               lu_ref_fini(&key->lct_reference);
+               atomic_set(&key->lct_used, 0);
+       }
        return result;
 }
 EXPORT_SYMBOL(lu_context_key_register);
        return result;
 }
 EXPORT_SYMBOL(lu_context_key_register);
@@ -1430,16 +1434,10 @@ void lu_context_key_degister(struct lu_context_key *key)
        atomic_dec(&key->lct_used);
        wait_var_event(&key->lct_used, atomic_read(&key->lct_used) == 0);
 
        atomic_dec(&key->lct_used);
        wait_var_event(&key->lct_used, atomic_read(&key->lct_used) == 0);
 
-       write_lock(&lu_keys_guard);
-       if (lu_keys[key->lct_index]) {
-               lu_keys[key->lct_index] = NULL;
+       if (!WARN_ON(lu_keys[key->lct_index] == NULL))
                lu_ref_fini(&key->lct_reference);
                lu_ref_fini(&key->lct_reference);
-       }
-       write_unlock(&lu_keys_guard);
 
 
-       LASSERTF(atomic_read(&key->lct_used) == 0,
-                "key has instances: %d\n",
-                atomic_read(&key->lct_used));
+       smp_store_release(&lu_keys[key->lct_index], NULL);
 }
 EXPORT_SYMBOL(lu_context_key_degister);
 
 }
 EXPORT_SYMBOL(lu_context_key_degister);