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

[Attilio Rao]

On Sun, Sep 9, 2012 at 8:15 PM, John Baldwin <jhb at freebsd.org> wrote:
> On 9/9/12 11:03 AM, Attilio Rao wrote:
>> On 8/2/12, Attilio Rao <attilio at freebsd.org> wrote:
>>> On 7/30/12, John Baldwin <jhb at freebsd.org> wrote:
>>
>> [ trimm ]
>>
>>>> --- //depot/projects/smpng/sys/kern/subr_turnstile.c        2012-06-04
>>>> 18:27:32.000000000 0000
>>>> +++ //depot/user/jhb/lock/kern/subr_turnstile.c     2012-06-05
>>>> 00:27:57.000000000 0000
>>>> @@ -684,6 +684,7 @@
>>>>     if (owner)
>>>>             MPASS(owner->td_proc->p_magic == P_MAGIC);
>>>>     MPASS(queue == TS_SHARED_QUEUE || queue == TS_EXCLUSIVE_QUEUE);
>>>> +   KASSERT(!TD_IS_IDLETHREAD(td), ("idle threads cannot block on locks"));
>>>>
>>>>     /*
>>>>      * If the lock does not already have a turnstile, use this thread's
>>>
>>> I'm wondering if we should also use similar checks in places doing
>>> adaptive spinning (including the TD_NO_SLEEPING check). Likely yes.
>>
>> So what do you think about this?
>
> This is isn't really good enough then.  An idle thread should not
> acquire any lock that isn't a spin lock.  Instead, you would be
> better off removing the assert I added above and adding an assert to
> mtx_lock(), rw_{rw}lock(), sx_{sx}lock(), lockmgr(), rm_{rw}lock() and
> all the try variants of those.

On a second thought, maybe this is not too bad, confining it in the
hard version of functions eventually.
Also, I think the checks may be good to go only on locking acquisition
operations, not all of them.
However, I would leave the check on sleepqueues as it also protects
idlethreads from sleep(9).

As a side node, also, I would change the output of the sleepqueues
panic to also print the wmesg, as Kostik pointed out.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein