OpenZFS 3746 - ZRLs are racy

Authored by: Will Andrews <will@freebsd.org>
Reviewed by: Boris Protopopov <bprotopopov@hotmail.com>
Reviewed by: Pavel Zakharov <pavel.zakha@gmail.com>
Reviewed by: Yuri Pankov <yuri.pankov@gmail.com>
Reviewed by: Justin T. Gibbs <gibbs@scsiguy.com>
Approved by: Matt Ahrens <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Ported-by: George Melikov <mail@gmelikov.ru>
OpenZFS-issue: https://www.illumos.org/issues/3746
OpenZFS-commit: https://github.com/openzfs/openzfs/commit/260af64
Closes #5625

diff --git a/module/zfs/zrlock.c b/module/zfs/zrlock.c
index 137875fe0694727116408cdfc03e0d8d4b045136..02795112f04279db47ca957fa8f029763cfeda05 100644 (file)
--- a/module/zfs/zrlock.c
+++ b/module/zfs/zrlock.c
@@ -20,7 +20,8 @@
  */
 /*
  * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2014, 2015 by Delphix. All rights reserved.
+ * Copyright 2016 The MathWorks, Inc. All rights reserved.
  */
 
 /*
@@ -72,37 +73,32 @@ zrl_destroy(zrlock_t *zrl)
 void
 zrl_add_impl(zrlock_t *zrl, const char *zc)
 {
-       uint32_t n = (uint32_t)zrl->zr_refcount;
-
-       while (n != ZRL_LOCKED) {
-               uint32_t cas = atomic_cas_32(
-                   (uint32_t *)&zrl->zr_refcount, n, n + 1);
-               if (cas == n) {
-                       ASSERT3S((int32_t)n, >=, 0);
+       for (;;) {
+               uint32_t n = (uint32_t)zrl->zr_refcount;
+               while (n != ZRL_LOCKED) {
+                       uint32_t cas = atomic_cas_32(
+                           (uint32_t *)&zrl->zr_refcount, n, n + 1);
+                       if (cas == n) {
+                               ASSERT3S((int32_t)n, >=, 0);
 #ifdef ZFS_DEBUG
-                       if (zrl->zr_owner == curthread) {
-                               DTRACE_PROBE2(zrlock__reentry,
-                                   zrlock_t *, zrl, uint32_t, n);
-                       }
-                       zrl->zr_owner = curthread;
-                       zrl->zr_caller = zc;
+                               if (zrl->zr_owner == curthread) {
+                                       DTRACE_PROBE2(zrlock__reentry,
+                                           zrlock_t *, zrl, uint32_t, n);
+                               }
+                               zrl->zr_owner = curthread;
+                               zrl->zr_caller = zc;
 #endif
-                       return;
+                               return;
+                       }
+                       n = cas;
                }
-               n = cas;
-       }
 
-       mutex_enter(&zrl->zr_mtx);
-       while (zrl->zr_refcount == ZRL_LOCKED) {
-               cv_wait(&zrl->zr_cv, &zrl->zr_mtx);
+               mutex_enter(&zrl->zr_mtx);
+               while (zrl->zr_refcount == ZRL_LOCKED) {
+                       cv_wait(&zrl->zr_cv, &zrl->zr_mtx);
+               }
+               mutex_exit(&zrl->zr_mtx);
        }
-       ASSERT3S(zrl->zr_refcount, >=, 0);
-       zrl->zr_refcount++;
-#ifdef ZFS_DEBUG
-       zrl->zr_owner = curthread;
-       zrl->zr_caller = zc;
-#endif
-       mutex_exit(&zrl->zr_mtx);
 }
 
 void