svn commit: r307994 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Andriy Gapon]

Author: avg
Date: Thu Oct 27 07:38:07 2016
New Revision: 307994
URL: https://svnweb.freebsd.org/changeset/base/307994

Log:
  3746 ZRLs are racy
  
  illumos/illumos-gate at 260af64db74a52d64de8c6c5f67dd0a71d228ca5
  https://github.com/illumos/illumos-gate/commit/260af64db74a52d64de8c6c5f67dd0a71d228ca5
  
  https://www.illumos.org/issues/3746
    From the original change log:
    It was possible for a reference to be added even with the lock held, and
    for references added just after a lock release to be lost.
    This bug was also independently found and reported in wesunsolve.net
    issues 6985013 6995524.
    In zrl_add(), always use an atomic operation to update the refcount.
    The mutex in the ZRL only guarantees that wakeups occur for waiters on the
    lock. It offers no protection against concurrent updates of the refcount.
    The only refcount transition that is safe to perform without an atomic
    operation is from ZRL_LOCKED back to 0, since this can only be performed
    by the thread which has the ZRL locked.
  
  Authored by: Will Andrews <will at freebsd.org>
  Reviewed by: Boris Protopopov <bprotopopov at hotmail.com>
  Reviewed by: Pavel Zakharov <pavel.zakha at gmail.com>
  Reviewed by: Yuri Pankov <yuri.pankov at gmail.com>
  Reviewed by: Justin T. Gibbs <gibbs at scsiguy.com>
  Approved by: Matt Ahrens <mahrens at delphix.com>
  Author: Youzhong Yang <yyang at mathworks.com>
  PR:		204037
  MFC after:	1 week

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zrlock.c
Directory Properties:
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zrlock.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zrlock.c	Thu Oct 27 07:11:31 2016	(r307993)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zrlock.c	Thu Oct 27 07:38:07 2016	(r307994)
@@ -21,6 +21,7 @@
 /*
  * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2014, 2015 by Delphix. All rights reserved.
+ * Copyright 2016 The MathWorks, Inc. All rights reserved.
  */
 
 /*
@@ -71,37 +72,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);
-#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;
+	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;
 #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