On Tuesday, October 19, 2010 9:37:15 am Alexander Motin wrote: > 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.
FWIW, my new interrupt thread code makes that binding explicit in the interrupt code and not in the scheduler. -- John Baldwin _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
