svn commit: r213985 - head/sys/sparc64/sparc64

[Alexander Motin]

John Baldwin wrote:
> On Monday, October 18, 2010 5:41:57 pm Alexander Motin wrote:
>> Marius Strobl wrote:
>>> On Mon, Oct 18, 2010 at 05:05:24PM -0400, John Baldwin wrote:
>>>> On Monday, October 18, 2010 4:52:24 pm Marius Strobl wrote:
>>>>> AFAICT this is not true; intr_event_handle() in sys/kern/kern_intr.c
>>>>> is what enters a critical section and f.e. on amd64 I don't see where
>>>>> anywhere in the path from ISR_VEC() to intr_execute_handlers()
>>>>> calling intr_event_handle() a critical section would be entered,
>>>>> which also means that in intr_execute_handlers() td_intr_nesting_level
>>>>> is incremented outside of a critical section.
>>>> Not all of the clock interrupts use intr_event_handle().  The local APIC
>>>> timer uses its own interrupt entry point on x86 for example and uses an
>>>> explicit critical section as a result.  I suspect the sparc64 tick interrupt
>>>> is closer to the local APIC timer case and doesn't use intr_event_handle().
>>> Correct; but still you can't say that the MD interrupt code enters a
>>> critical section in general, neither is incrementing td_intr_nesting_level
>>> in intr_execute_handlers() protected by a critical section.
>>>
>>>> The fact that some clock interrupts do use intr_event_handle() (e.g. the
>>>> atrtc driver on x86 now) does indicate that the low-level interrupt code
>>>> probably does not belong in the time events code but in the caller.
>>> Well, I agree that entering a critical section in the time events
>>> code would mean entering a nested critical section unnecessarily in
>>> case the clock driver uses a regular "fast" interrupt handler and
>>> that should be avoided. Still I don't think the event time front-end
>>> actually should need to worry about wrapping the callback in a
>>> critical section.
>> Interrupt frame, required for hard-/stat-/profclock() operation is
>> stored in curthread. So critical section is effectively mandatory there
>> now. Correct td_intr_nesting_level value is also important for proper
>> interrupt threads scheduling - one more reason to have critical section
>> there. It is indeed strange that td_intr_nesting_level in
>> intr_event_handle() is not covered by critical section, but probably it
>> should.
> 
> I don't see why the interrupt frame logic requires bumping
> td_intr_nesting_level. 

It doesn't. It was two different sentences. I've just noticed that
td_intr_nesting_level used by SCHED_ULE to bind interrupt threads on
their CPUs. I agree that it may be not a very good idea, but it is what
we have now.

-- 
Alexander Motin