Date: Tue, 21 Jul 2015 10:39:22 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Adrian Chadd <adrian.chadd@gmail.com> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@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> In-Reply-To: <CAJ-VmokSbajFxOKPU=u4AQgPmtPqJXvzfbp4%2Bhrcd-PsCYq7PA@mail.gmail.com> References: <201507040654.t646sGO7044196@repo.freebsd.org> <CAJ-VmokSbajFxOKPU=u4AQgPmtPqJXvzfbp4%2Bhrcd-PsCYq7PA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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=<value optimized out>) at > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_shutdown.c:634 > #11 0xc06f449b in sleepq_add (wchan=0xc0b3e8e4, wmesg=<value optimized > out>, flags=<value optimized out>, queue=<value optimized out>) at > /usr/home/adrian/work/freebsd/head/src/sys/kern/subr_sleepqueue.c:308 > #12 0xc06b167b in _sx_xlock_hard (sx=0xc0b3e8e4, tid=<value optimized > out>, opts=<value optimized out>, 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=<value optimized > 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=<value optimized out>) 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=<value optimized out>, arg2=<value optimized out>, req=<value > optimized out>, tracker=0xeaa61ad0) at > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_sysctl.c:170 > #17 0xc06b5531 in sysctl_root (arg1=<value optimized out>, arg2=<value > optimized out>) at > /usr/home/adrian/work/freebsd/head/src/sys/kern/kern_sysctl.c:1692 > #18 0xc06b5ac2 in userland_sysctl (td=<value optimized out>, > name=<value optimized out>, namelen=2, old=<value optimized out>, > oldlenp=<value optimized out>, inkernel=<value optimized out>, > new=<value optimized out>, newlen=<value optimized out>, retval=0x12, > flags=<value optimized out>) 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=<value optimized out>) 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 <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150721083922.GB6736>