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

[Davide Italiano]

On Mon, Jul 30, 2012 at 10:48 PM, Attilio Rao <attilio at freebsd.org> wrote:
> On Mon, Jul 30, 2012 at 3:59 PM, Konstantin Belousov
> <kostikbel at gmail.com> wrote:
>> On Mon, Jul 30, 2012 at 03:51:22PM +0100, Attilio Rao wrote:
>>> On 7/30/12, Konstantin Belousov <kostikbel at gmail.com> 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.
>>>
>>> Yes, however I would use curthread_pflags* KPI within
>>> THREAD_NO_SLEEPING() as this name is much more explicit.
>>>
>> Sure, hiding it in THREAD_NO_SLEEPING (THREAD_NO_SLEEP_ENTER/LEAVE ?)
>> is the way to use curthread_pflags_set there.
>>
>> As a second though, on the other hand, is it safe to modify td_flags
>> from the interrupt context at all ? Probably yes if interrupt handler
>> always leave td_pflags in the same state on leave as it was on entry,
>> but couldn't too smart compiler cause inconsistent view of td_pflags
>> inside the handler ?
>
> Can you think of any? Because I cannot think of a case where a nested
> interrupt can messup with already compiled code, unless it leaks a
> cleanup.
>
> I was more worried about the compiler reordering operations before
> locking could really see it, but I think in this case the functions
> call to sleepqueue (at least) works as a sequence point so we are
> safe.
>
>>
>>> > Also, I wonder, should you assert somehow that direct dispatch cannot block
>>> > as well ?
>>>
>>> Yes, it would be optimal, but I don't think we have a flag for that
>>> right now, do we?
>>
>> I am not aware of such flag, this might be a good reason to introduce it,
>> if issue about td_pflags is just a product of my imagination.
>
> I think you should be good to go. Do you plan to work on such a patch?

I may work on that as final part of my GSoC work considering I've an
interest in this.
Though, I could need some guidance and help in review. Can you provide these?

>
> Thanks,
> Attilio
>
>
> --
> Peace can only be achieved by understanding - A. Einstein

Thanks,

Davide