From owner-svn-src-head@freebsd.org Tue Jul 21 08:39:29 2015 Return-Path: Delivered-To: svn-src-head@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 173F99A7847; Tue, 21 Jul 2015 08:39:29 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wi0-x22f.google.com (mail-wi0-x22f.google.com [IPv6:2a00:1450:400c:c05::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id B9D1E1B89; Tue, 21 Jul 2015 08:39:28 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by wicgb10 with SMTP id gb10so48376527wic.1; Tue, 21 Jul 2015 01:39:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=yNf+tXPWZajWHnLZ7ustMkmcLxLubv2tyyLiMsaWBeg=; b=YwK9HANiUKFXEsVQEMOa7Lpi2hkeIQr7K668DyOiRM20ng56KXVJXAc0aztVHxSe2H uhC9R0Q9eDtN+BHu9NQsn3F2VmmRyPCghOpoSxPuLrVHqMxTOhDaDp/8afQ5HKN90j9E BWOZZhZ73/4GMS60U1exiZlw/q9FKLgmMdTRvkTTpcOQyopU+S/CAmaDf+dgqODFrGJ0 cX7KO145O6TuOK+pz+3lcWrpDPiBqR8ZlA+GpPsHQ4Cr2hjrzreuT14vXgIcoPilnqug t1w8N19Xt1tS+hGna8U/MwaeQ6gPdiAQBLDh8cJ3+lXEnmx5SuLF/mEVytxWt1wgMxze K2DA== X-Received: by 10.194.172.130 with SMTP id bc2mr69335020wjc.85.1437467967119; Tue, 21 Jul 2015 01:39:27 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by smtp.gmail.com with ESMTPSA id ez4sm15612052wid.14.2015.07.21.01.39.24 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 21 Jul 2015 01:39:25 -0700 (PDT) Date: Tue, 21 Jul 2015 10:39:22 +0200 From: Mateusz Guzik To: Adrian Chadd Cc: "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" , jhb@freebsd.org Subject: Re: svn commit: r285125 - in head/sys: kern sys Message-ID: <20150721083922.GB6736@dft-labs.eu> References: <201507040654.t646sGO7044196@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 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: Tue, 21 Jul 2015 08:39:29 -0000 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(...); Turns out it's the rm_rlock itself which must not sleep and you have to rm_try_rlock instead. Now, why does not rm_rlock panic outright when passed a sleepable lock? Further, that's a rather unfortunate property. With only skimming through rmlock implementation, I get why you would want to actualy not sleep in rm_rlock. So how about a helper (say rm_rlock_sleepable) which would be roughly: void _rm_rlock_sleepable(struct rmlock *rm, struct rm_priotracker *tracker) { MPASS(rm->lock_object.lo_flags & LO_SLEEPABLE); while (rm_try_rlock(rm, tracker) == 0) { sx_slock(&rm->rm_lock_sx); sx_sunlock(&rm->rm_lock_sx); } } 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. -- Mateusz Guzik