svn commit: r238907 - projects/calloutng/sys/kern

[Attilio Rao]

On Mon, Jul 30, 2012 at 9:52 PM, Attilio Rao <attilio at freebsd.org> wrote:
> On Mon, Jul 30, 2012 at 4:49 PM, John Baldwin <jhb at 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 at freebsd.org> wrote:
>>> > > On Mon, Jul 30, 2012 at 4:02 PM, Attilio Rao <attilio at 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