From owner-svn-src-all@freebsd.org Tue Jul 21 18:50:35 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C2D129A7269; Tue, 21 Jul 2015 18:50:35 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 835C618DA; Tue, 21 Jul 2015 18:50:35 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (75-48-78-19.lightspeed.cncrca.sbcglobal.net [75.48.78.19]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id B8E26B941; Tue, 21 Jul 2015 14:50:33 -0400 (EDT) From: John Baldwin To: Mateusz Guzik Cc: Adrian Chadd , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Subject: Re: svn commit: r285125 - in head/sys: kern sys Date: Tue, 21 Jul 2015 11:50:12 -0700 Message-ID: <3863130.vz23U50G0A@ralph.baldwin.cx> User-Agent: KMail/4.14.3 (FreeBSD/10.1-STABLE; KDE/4.14.3; amd64; ; ) In-Reply-To: <20150721083922.GB6736@dft-labs.eu> References: <201507040654.t646sGO7044196@repo.freebsd.org> <20150721083922.GB6736@dft-labs.eu> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 21 Jul 2015 14:50:33 -0400 (EDT) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Jul 2015 18:50:35 -0000 On Tuesday, July 21, 2015 10:39:22 AM Mateusz Guzik wrote: > Cc'ing jhb@ who made relevant changes to rmlocks > > On Mon, Jul 20, 2015 at 11:21:30PM -0700, Adrian Chadd wrote: > > this happend whilst doing 'sysctl -vmstat 1' in one window, 'top' in > > another window, and 'kldunload uhci' as root: > > > > Unread portion of the kernel message buffer: > > uhci5: detached > > panic: sleepq_add: td 0xc75d06a0 to sleep on wchan 0xc0b3e8e4 with > > sleeping prohibited > > > [..] > > > #10 0xc06a882c in kassert_panic (fmt=) at > > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_shutdown.c:634 > > #11 0xc06f449b in sleepq_add (wchan=0xc0b3e8e4, wmesg= > out>, flags=, queue=) at > > /usr/home/adrian/work/freebsd/head/src/sys/kern/subr_sleepqueue.c:308 > > #12 0xc06b167b in _sx_xlock_hard (sx=0xc0b3e8e4, tid= > out>, opts=, file=0x0, line=18) at > > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_sx.c:697 > > #13 0xc06b0841 in _sx_xlock (sx=0xc0b3e8e4, opts= > out>, file=0xc09f2d35 > > "/usr/home/adrian/work/freebsd/head/src/sys/kern/kern_rmlock.c", > > line=411) at sx.h:154 > > #14 0xc06a4510 in _rm_rlock (rm=0xc0b3e8cc, tracker=0xeaa61ad0, > > trylock=) at > > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_rmlock.c:411 > > #15 0xc06a4e2d in _rm_rlock_debug (rm=0xc0b3e8cc, tracker=0xeaa61ad0, > > trylock=0) at /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_rmlock.c:665 > > #16 0xc06b5da4 in sysctl_root_handler_locked (oid=0xc0a6ee20, > > arg1=, arg2=, req= > optimized out>, tracker=0xeaa61ad0) at > > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_sysctl.c:170 > > #17 0xc06b5531 in sysctl_root (arg1=, arg2= > optimized out>) at > > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_sysctl.c:1692 > > #18 0xc06b5ac2 in userland_sysctl (td=, > > name=, namelen=2, old=, > > oldlenp=, inkernel=, > > new=, newlen=, retval=0x12, > > flags=) at > > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_sysctl.c:1797 > > #19 0xc06b5908 in sys___sysctl (uap=0xeaa61ca8) at > > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_sysctl.c:1724 > > #20 0xc096aaee in syscall (frame=) at subr_syscall.c:133 > > #21 0xc0955c5c in Xint0x80_syscall () at > > /usr/home/adrian/work/freebsd/head/src/sys/i386/i386/exception.s:278 > > rmlock(9) states: > Sleepable read-mostly locks are created by passing RM_SLEEPABLE to > rm_init_flags(). Unlike normal read-mostly locks, sleepable read-mostly > locks follow the same lock ordering rules as sx(9) locks. Sleepable > read-mostly locks do not propagate priority to writers, but they do > propagate priority to readers. Writers are permitted to sleep while > holding a read-mostly lock, but readers are not. Unlike other sleepable > locks such as sx(9) locks, readers must use try operations on other > sleepable locks to avoid sleeping. > > May be that's my bad English, but I read that: > rm_rlock(...); > /* can't sleep here */ > rm_runlock(...); That is correct. > Turns out it's the rm_rlock itself which must not sleep and you have to > rm_try_rlock instead. Hmm, I think instead, the THREAD_NO_SLEEPING in rm_rlock() just needs to be moved. Or rather, rm_rlock_hard() needs to do THREAD_SLEEPING_OK around the sx_xlock and then THREAD_NO_SLEEPING afterwards. Something like this: Index: kern/kern_rmlock.c =================================================================== --- kern/kern_rmlock.c (revision 284344) +++ kern/kern_rmlock.c (working copy) @@ -407,9 +407,11 @@ return (0); } } else { - if (rm->lock_object.lo_flags & LO_SLEEPABLE) + if (rm->lock_object.lo_flags & LO_SLEEPABLE) { + THREAD_SLEEPING_OK(); sx_xlock(&rm->rm_lock_sx); - else + THREAD_NO_SLEEPING(); + } else mtx_lock(&rm->rm_lock_mtx); } > As for the usage here, sysctl seems like a natural consumer for the lock > since after the kernel boots multiuser tree change is a "once in a year" > event. This offers a very minor performance gain as well. The patch can > be reverted back to a mere sx lock, but currently sysctl is the only > in-tree consumer of sleepable rmlocks, so I would argue it is beneficial > to keep it as a user if only to know the feature is somewhat > operational. RM_SLEEPABLE was first added as a feature for an out-of-tree consumer (Isilon). I merely documented it when adding WITNESS support to rmlocks and fleshing out assertions, etc. -- John Baldwin