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

[Attilio Rao]

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?

Thanks,
Attilio


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