Date: Wed, 22 Jul 2015 06:33:24 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: Adrian Chadd <adrian.chadd@gmail.com>, "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> Subject: Re: svn commit: r285125 - in head/sys: kern sys Message-ID: <20150722043323.GA23614@dft-labs.eu> In-Reply-To: <3863130.vz23U50G0A@ralph.baldwin.cx> References: <201507040654.t646sGO7044196@repo.freebsd.org> <CAJ-VmokSbajFxOKPU=u4AQgPmtPqJXvzfbp4%2Bhrcd-PsCYq7PA@mail.gmail.com> <20150721083922.GB6736@dft-labs.eu> <3863130.vz23U50G0A@ralph.baldwin.cx>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jul 21, 2015 at 11:50:12AM -0700, John Baldwin wrote:
> 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=<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(...);
>
> 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);
> }
>
Works for me, I was reluctant to do this myself, hence the "wrapper"
approach.
Thanks.
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150722043323.GA23614>