From owner-svn-src-head@freebsd.org Wed Nov 4 21:18:09 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 3C879466250; Wed, 4 Nov 2020 21:18:09 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CRKJ10ngsz4H5F; Wed, 4 Nov 2020 21:18:09 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id EF8B213F16; Wed, 4 Nov 2020 21:18:08 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 0A4LI8pL077090; Wed, 4 Nov 2020 21:18:08 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 0A4LI8wX077086; Wed, 4 Nov 2020 21:18:08 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <202011042118.0A4LI8wX077086@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Wed, 4 Nov 2020 21:18:08 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r367341 - in head/sys: kern sys X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: in head/sys: kern sys X-SVN-Commit-Revision: 367341 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Nov 2020 21:18:09 -0000 Author: mjg Date: Wed Nov 4 21:18:08 2020 New Revision: 367341 URL: https://svnweb.freebsd.org/changeset/base/367341 Log: rms: fixup concurrent writer handling and add more features Previously the code had one wait channel for all pending writers. This could result in a buggy scenario where after a writer switches the lock mode form readers to writers goes off CPU, another writer queues itself and then the last reader wakes up the latter instead of the former. Use a separate channel. While here add features to reliably detect whether curthread has the lock write-owned. This will be used by ZFS. Modified: head/sys/kern/kern_rmlock.c head/sys/sys/_rmlock.h head/sys/sys/rmlock.h Modified: head/sys/kern/kern_rmlock.c ============================================================================== --- head/sys/kern/kern_rmlock.c Wed Nov 4 20:15:14 2020 (r367340) +++ head/sys/kern/kern_rmlock.c Wed Nov 4 21:18:08 2020 (r367341) @@ -878,10 +878,15 @@ db_show_rm(const struct lock_object *lock) * problem at some point. The easiest way to lessen it is to provide a bitmap. */ +#define RMS_NOOWNER ((void *)0x1) +#define RMS_TRANSIENT ((void *)0x2) +#define RMS_FLAGMASK 0xf + void rms_init(struct rmslock *rms, const char *name) { + rms->owner = RMS_NOOWNER; rms->writers = 0; rms->readers = 0; mtx_init(&rms->mtx, name, NULL, MTX_DEF | MTX_NEW); @@ -922,6 +927,7 @@ rms_rlock(struct rmslock *rms) { WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__); + MPASS(atomic_load_ptr(&rms->owner) != curthread); critical_enter(); zpcpu_set_protected(rms->readers_influx, 1); @@ -941,6 +947,8 @@ int rms_try_rlock(struct rmslock *rms) { + MPASS(atomic_load_ptr(&rms->owner) != curthread); + critical_enter(); zpcpu_set_protected(rms->readers_influx, 1); __compiler_membar(); @@ -1054,23 +1062,33 @@ rms_wlock(struct rmslock *rms) { WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, __func__); + MPASS(atomic_load_ptr(&rms->owner) != curthread); mtx_lock(&rms->mtx); rms->writers++; if (rms->writers > 1) { - msleep(&rms->writers, &rms->mtx, (PUSER - 1) | PDROP, + msleep(&rms->owner, &rms->mtx, (PUSER - 1), mtx_name(&rms->mtx), 0); MPASS(rms->readers == 0); - return; + KASSERT(rms->owner == RMS_TRANSIENT, + ("%s: unexpected owner value %p\n", __func__, + rms->owner)); + goto out_grab; } + KASSERT(rms->owner == RMS_NOOWNER, + ("%s: unexpected owner value %p\n", __func__, rms->owner)); + rms_wlock_switch(rms); - if (rms->readers > 0) - msleep(&rms->writers, &rms->mtx, (PUSER - 1) | PDROP, + if (rms->readers > 0) { + msleep(&rms->writers, &rms->mtx, (PUSER - 1), mtx_name(&rms->mtx), 0); - else - mtx_unlock(&rms->mtx); + } + +out_grab: + rms->owner = curthread; + mtx_unlock(&rms->mtx); MPASS(rms->readers == 0); } @@ -1079,12 +1097,27 @@ rms_wunlock(struct rmslock *rms) { mtx_lock(&rms->mtx); + KASSERT(rms->owner == curthread, + ("%s: unexpected owner value %p\n", __func__, rms->owner)); MPASS(rms->writers >= 1); MPASS(rms->readers == 0); rms->writers--; - if (rms->writers > 0) - wakeup_one(&rms->writers); - else + if (rms->writers > 0) { + wakeup_one(&rms->owner); + rms->owner = RMS_TRANSIENT; + } else { wakeup(&rms->readers); + rms->owner = RMS_NOOWNER; + } mtx_unlock(&rms->mtx); +} + +void +rms_unlock(struct rmslock *rms) +{ + + if (rms_wowned(rms)) + rms_wunlock(rms); + else + rms_runlock(rms); } Modified: head/sys/sys/_rmlock.h ============================================================================== --- head/sys/sys/_rmlock.h Wed Nov 4 20:15:14 2020 (r367340) +++ head/sys/sys/_rmlock.h Wed Nov 4 21:18:08 2020 (r367341) @@ -72,6 +72,7 @@ struct rm_priotracker { struct rmslock { struct mtx mtx; + struct thread *owner; int writers; int readers; int *readers_pcpu; Modified: head/sys/sys/rmlock.h ============================================================================== --- head/sys/sys/rmlock.h Wed Nov 4 20:15:14 2020 (r367340) +++ head/sys/sys/rmlock.h Wed Nov 4 21:18:08 2020 (r367341) @@ -140,16 +140,33 @@ int rms_try_rlock(struct rmslock *rms); void rms_runlock(struct rmslock *rms); void rms_wlock(struct rmslock *rms); void rms_wunlock(struct rmslock *rms); +void rms_unlock(struct rmslock *rms); +static inline int +rms_wowned(struct rmslock *rms) +{ + + return (rms->owner == curthread); +} + /* - * Writers are not explicitly tracked, thus until that changes the best we can - * do is indicate the lock is taken for writing by *someone*. + * Only valid to call if you hold the lock in some manner. */ static inline int -rms_wowned(struct rmslock *rms) +rms_rowned(struct rmslock *rms) { - return (rms->writers > 0); + return (rms->readers > 0); +} + +static inline int +rms_owned_any(struct rmslock *rms) +{ + + if (rms_wowned(rms)) + return (1); + + return (rms_rowned(rms)); } #endif /* _KERNEL */