Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 19 Oct 2010 09:46:56 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Alexander Motin <mav@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Marius Strobl <marius@alchemy.franken.de>
Subject:   Re: svn commit: r213985 - head/sys/sparc64/sparc64
Message-ID:  <201010190946.56535.jhb@freebsd.org>
In-Reply-To: <4CBD9F0B.80701@FreeBSD.org>
References:  <201010171646.o9HGks2U038501@svn.freebsd.org> <201010190836.37272.jhb@freebsd.org> <4CBD9F0B.80701@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201010190946.56535.jhb>