3746 ZRLs are racy

illumos/illumos-gate@260af64db7
260af64db7

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@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>
Author: Youzhong Yang <yyang@mathworks.com>
This commit is contained in:
Andriy Gapon 2016-10-27 07:11:31 +00:00
parent 760c70cd25
commit 0a682531f4

View File

@ -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);
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);
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;
}
zrl->zr_owner = curthread;
zrl->zr_caller = zc;
#endif
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