Date: Mon, 30 Jul 2012 22:00:20 +0100 From: Attilio Rao <attilio@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, Davide Italiano <davide@freebsd.org>, src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern Message-ID: <CAJ-FndDnO7wjnWPV0tTu%2BUGHjsxa3YDarMxmyei3ZmjLAFvRkQ@mail.gmail.com> In-Reply-To: <CAJ-FndBj8tpC_BJXs_RH8sG2TBG8yA=Lxu3-GTVT9Ap_zOCuVQ@mail.gmail.com> References: <201207301350.q6UDobCI099069@svn.freebsd.org> <CAJ-FndBmXkyJJ=fCkEpVm84E56A2_EoM6kbch03e4RMEM6WCGQ@mail.gmail.com> <20120730143943.GY2676@deviant.kiev.zoral.com.ua> <201207301149.43458.jhb@freebsd.org> <CAJ-FndBj8tpC_BJXs_RH8sG2TBG8yA=Lxu3-GTVT9Ap_zOCuVQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jul 30, 2012 at 9:52 PM, Attilio Rao <attilio@freebsd.org> wrote: > On Mon, Jul 30, 2012 at 4:49 PM, John Baldwin <jhb@freebsd.org> wrote: >> On Monday, July 30, 2012 10:39:43 am Konstantin Belousov wrote: >>> On Mon, Jul 30, 2012 at 03:24:26PM +0100, Attilio Rao wrote: >>> > On 7/30/12, Davide Italiano <davide@freebsd.org> wrote: >>> > > On Mon, Jul 30, 2012 at 4:02 PM, Attilio Rao <attilio@freebsd.org> >> wrote: >>> > > Thanks for the comment, Attilio. >>> > > Yes, it's exactly what you thought. If direct flag is equal to one >>> > > you're sure you're processing a callout which runs directly from >>> > > hardware interrupt context. In this case, the running thread cannot >>> > > sleep and it's likely you have TDP_NOSLEEPING flags set, failing the >>> > > KASSERT() in THREAD_NO_SLEEPING() and leading to panic if kernel is >>> > > compiled with INVARIANTS. >>> > > In case you're running from SWI context (direct equals to zero) code >>> > > remains the same as before. >>> > > I think what I'm doing works due the assumption thread running never >>> > > sleeps. Do you suggest some other way to handle this? >>> > >>> > Possibly the quicker way to do this is to have a way to deal with the >>> > TDP_NOSLEEPING flag in recursed way, thus implement the same logic as >>> > VFS_LOCK_GIANT() does, for example. >>> > You will need to change the few callers of THREAD_NO_SLEEPING(), but >>> > the patch should be no longer than 10/15 lines. >>> >>> There are already curthread_pflags_set/restore KPI designed exactly to >> handle >>> nested private thread flags. >>> >>> Also, I wonder, should you assert somehow that direct dispatch cannot block >>> as well ? >> >> Hmm, I have a nested TDP_NOSLEEPING already (need it to fix an issue in >> rmlocks). It uses a count though as the flag is set during rm_rlock() and >> released during rm_runlock(). I don't think it could use a set/restore KPI as >> there is no good place to store the state. > > Our stock rmlocks don't seem to use TDP_NOSLEEPING/THREAD_NO_SLEEPING > so I'm not entirely sure about the case you were trying to fix, can > you show the patch? I think I can see what you did. Did you add TDP_NOSLEEPING when acquiring the first rmlock and drop when the last was released. This is a nice property, in general exendible to all our blocking primitives, really. Right now some sort of similar check is enforced by WITNESS, but I can see a value in cases where you want to test a kernel with INVARIANTS but without WITNESS (it is not a matter of performance, it is just that sometimes you cannot reproduce some specific races with WITNESS on, while you can do it with WITNESS off, so it is funny to note how sometimes WITNESS should be just dropped for some locking issues). Attilio -- Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndDnO7wjnWPV0tTu%2BUGHjsxa3YDarMxmyei3ZmjLAFvRkQ>