PMU and timer interrupts

PMU and timer interrupts

[Stephane Eranian]

Hello,

It was recently pointed out to me that the PMU interrupt
vector is set to 0xee just below the timer interrupt at 0xef.
As such, the timer interrupt has higher priority than the PMU
interrupt. This would make sense except that a side effect is
that it is not possible to collect samples from within the timer
interrupt, i.e. we have a blind spot. Another side effect is
that events happening during the timer handler, may be falsely
attributed to the point where we write the EOI register.

I looked at the timer_interrupt() path, and I did not see anything special
that would prevent us from swapping the interrupt vectors thereby removing
the blind spot we have.

Does anybody see a problem with this?

-- 
-Stephane

Re: PMU and timer interrupts

[Jack Steiner]

On Thu, Feb 09, 2006 at 09:16:48AM -0800, Stephane Eranian wrote:
> Hello,
> 
> It was recently pointed out to me that the PMU interrupt
> vector is set to 0xee just below the timer interrupt at 0xef.
> As such, the timer interrupt has higher priority than the PMU
> interrupt. This would make sense except that a side effect is
> that it is not possible to collect samples from within the timer
> interrupt, i.e. we have a blind spot. Another side effect is
> that events happening during the timer handler, may be falsely
> attributed to the point where we write the EOI register.

If your intent is to allow a PMU interrupt to interrupt the timer 
interrupt code, don't you need to put the PMU interrupt into a
higher priority class than the timer interrupt? Interrupt vectors
are grouped into 16 interrupt classes. An in-service interrupt masks
all other interrupts in the same class. (See the TPR register).

For example, PMU = 0xe0 & timer = 0xdf?
(This obviously ripples thu additional interrupt numbers....)
 
> I looked at the timer_interrupt() path, and I did not see anything special
> that would prevent us from swapping the interrupt vectors thereby removing
> the blind spot we have.
> 
> Does anybody see a problem with this?
> 
> -- 
> -Stephane
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.

RE: PMU and timer interrupts

[Luck, Tony]

>It was recently pointed out to me that the PMU interrupt
>vector is set to 0xee just below the timer interrupt at 0xef.
>As such, the timer interrupt has higher priority than the PMU
>interrupt. This would make sense except that a side effect is
>that it is not possible to collect samples from within the timer
>interrupt, i.e. we have a blind spot. Another side effect is
>that events happening during the timer handler, may be falsely
>attributed to the point where we write the EOI register.
>
>I looked at the timer_interrupt() path, and I did not see anything special
>that would prevent us from swapping the interrupt vectors thereby removing
>the blind spot we have.
>
>Does anybody see a problem with this?

How much would this help?  Both 0xEF and 0xEE are part of the
same priority class (see table 5-8 on p. 2-112 of SDM).

-Tony

Re: PMU and timer interrupts

[Stephane Eranian]

Tony, Jack,

On Thu, Feb 09, 2006 at 10:20:18AM -0800, Luck, Tony wrote:
> >It was recently pointed out to me that the PMU interrupt
> >vector is set to 0xee just below the timer interrupt at 0xef.
> >As such, the timer interrupt has higher priority than the PMU
> >interrupt. This would make sense except that a side effect is
> >that it is not possible to collect samples from within the timer
> >interrupt, i.e. we have a blind spot. Another side effect is
> >that events happening during the timer handler, may be falsely
> >attributed to the point where we write the EOI register.
> >
> >I looked at the timer_interrupt() path, and I did not see anything special
> >that would prevent us from swapping the interrupt vectors thereby removing
> >the blind spot we have.
> >
> >Does anybody see a problem with this?
> 
> How much would this help?  Both 0xEF and 0xEE are part of the
> same priority class (see table 5-8 on p. 2-112 of SDM).
> 
Correct me if I am wrong, but the class masking happens only with TPR
masking. So if we had timer=0xee, PMU=0xef and if the kernel does not
raise the TPR class masking to the priority class of 0xef/0xee, then
we could still get a PMU interrupt while executing the timer handler.


-- 
-Stephane

Re: PMU and timer interrupts

[Jack Steiner]

On Thu, Feb 09, 2006 at 10:39:49AM -0800, Stephane Eranian wrote:
> Tony, Jack,
> 
> On Thu, Feb 09, 2006 at 10:20:18AM -0800, Luck, Tony wrote:
> > >It was recently pointed out to me that the PMU interrupt
> > >vector is set to 0xee just below the timer interrupt at 0xef.
> > >As such, the timer interrupt has higher priority than the PMU
> > >interrupt. This would make sense except that a side effect is
> > >that it is not possible to collect samples from within the timer
> > >interrupt, i.e. we have a blind spot. Another side effect is
> > >that events happening during the timer handler, may be falsely
> > >attributed to the point where we write the EOI register.
> > >
> > >I looked at the timer_interrupt() path, and I did not see anything special
> > >that would prevent us from swapping the interrupt vectors thereby removing
> > >the blind spot we have.
> > >
> > >Does anybody see a problem with this?
> > 
> > How much would this help?  Both 0xEF and 0xEE are part of the
> > same priority class (see table 5-8 on p. 2-112 of SDM).
> > 
> Correct me if I am wrong, but the class masking happens only with TPR
> masking. So if we had timer=0xee, PMU=0xef and if the kernel does not
> raise the TPR class masking to the priority class of 0xef/0xee, then
> we could still get a PMU interrupt while executing the timer handler.
> 

What you say is true. However, the kernel does use the TPR to enforce 
priority classes:

	void
	ia64_handle_irq (ia64_vector vector, struct pt_regs *regs)
	{
		...
		saved_tpr = ia64_getreg(_IA64_REG_CR_TPR);
		while (vector != IA64_SPURIOUS_INT_VECTOR) {
			if (!IS_RESCHEDULE(vector)) {
				ia64_setreg(_IA64_REG_CR_TPR, vector);
				__do_IRQ(local_vector_to_irq(vector), regs);
		....
	}

Use of priority classes prevents excessive recursion. Without it, as a worse case,
	- a priority 16 interrupt could be interrupted by a priority 17
	- which is interrupted by an 18
	- which is interrupted by an 19
	- which is interrupted by an 20
	- etc. and the kernel stack goes BOOM   :-)

Priority classes limits the recursion to 15 levels at most. 

Re: PMU and timer interrupts

[Stephane Eranian]

Jack,

On Thu, Feb 09, 2006 at 01:10:22PM -0600, Jack Steiner wrote:
> On Thu, Feb 09, 2006 at 10:39:49AM -0800, Stephane Eranian wrote:
> > Tony, Jack,
> > 
> > On Thu, Feb 09, 2006 at 10:20:18AM -0800, Luck, Tony wrote:
> > > >It was recently pointed out to me that the PMU interrupt
> > > >vector is set to 0xee just below the timer interrupt at 0xef.
> > > >As such, the timer interrupt has higher priority than the PMU
> > > >interrupt. This would make sense except that a side effect is
> > > >that it is not possible to collect samples from within the timer
> > > >interrupt, i.e. we have a blind spot. Another side effect is
> > > >that events happening during the timer handler, may be falsely
> > > >attributed to the point where we write the EOI register.
> > > >
> > > >I looked at the timer_interrupt() path, and I did not see anything special
> > > >that would prevent us from swapping the interrupt vectors thereby removing
> > > >the blind spot we have.
> > > >
> > > >Does anybody see a problem with this?
> > > 
> > > How much would this help?  Both 0xEF and 0xEE are part of the
> > > same priority class (see table 5-8 on p. 2-112 of SDM).
> > > 
> > Correct me if I am wrong, but the class masking happens only with TPR
> > masking. So if we had timer=0xee, PMU=0xef and if the kernel does not
> > raise the TPR class masking to the priority class of 0xef/0xee, then
> > we could still get a PMU interrupt while executing the timer handler.
> > 
> 
> What you say is true. However, the kernel does use the TPR to enforce 
> priority classes:
> 
> 	void
> 	ia64_handle_irq (ia64_vector vector, struct pt_regs *regs)
> 	{
> 		...
> 		saved_tpr = ia64_getreg(_IA64_REG_CR_TPR);
> 		while (vector != IA64_SPURIOUS_INT_VECTOR) {
> 			if (!IS_RESCHEDULE(vector)) {
> 				ia64_setreg(_IA64_REG_CR_TPR, vector);
> 				__do_IRQ(local_vector_to_irq(vector), regs);

Ok, now I understand better where you were coming from. So what would be the
problem in push PMU interrupt in the next class up?

Re: PMU and timer interrupts

[Stephane Eranian]

Jack, Tony,

Suposing I move up the PERFMON_VECTOR to 0xf1, it will be in the
top priority class (15) with the following vectors:

> 	#define IA64_MCA_WAKEUP_VECTOR          0xf0    /* MCA wakeup (must be >MCA_RENDEZ_VECTOR) */
> 	#define IA64_IPI_RESCHEDULE             0xfd    /* SMP reschedule */
> 	#define IA64_IPI_VECTOR                 0xfe    /* inter-processor interrupt vector */

The impact will be that, while servicing a PMU interrupt, I will not be able to get:
	-  an MCA interrupt
	-  a reschedule IPI
	-  a generic IPI

Today, the PMU interrupt handler runs with interrupts disabled, as such the situation would not
be different. In fact, we could skip masking/unmasking because we would be in the top class +
TPR masking done by the kernel on interrupt.

Anybody, has a problem with this?

-- 
-Stephane

Re: PMU and timer interrupts

[Jes Sorensen]

>>>>> "Stephane" = Stephane Eranian <eranian@hpl.hp.com> writes:

Stephane> Jack, Tony, Suposing I move up the PERFMON_VECTOR to 0xf1,
Stephane> it will be in the top priority class (15) with the following
Stephane> vectors:

Hi Stephane,

I admit I don't know much about how this works, but would it be
possible to move IA64_MCA_RENDEZ_VECTOR and IA64_TIMER_VECTOR to the
0xdX group? I looked quickly at a 64-way box internally and it only
had stuff assigned up to irq 70 (70 decimal that is) even with quit a
lot of storage attached.

Cheers,
Jes

RE: PMU and timer interrupts

[Chen, Kenneth W]

Stephane Eranian wrote on Friday, February 10, 2006 5:35 AM
> Suposing I move up the PERFMON_VECTOR to 0xf1, it will be in the
> top priority class (15) with the following vectors:
> 
> Today, the PMU interrupt handler runs with interrupts disabled,
> as such the situation would not be different. In fact, we could
> skip masking/unmasking because we would be in the top class +
> TPR masking done by the kernel on interrupt.
> 
> Anybody, has a problem with this?

If your goal is to be able to sample timer interrupt with PMU, you
won't get it even after you move PMU vector one class up.  Because
just like PMU vector, timer interrupt handler is run with interrupt
off all the through.

- Ken

Re: PMU and timer interrupts

[Stephane Eranian]

Ken,

On Fri, Feb 10, 2006 at 12:33:20PM -0800, Chen, Kenneth W wrote:
> Stephane Eranian wrote on Friday, February 10, 2006 5:35 AM
> > Suposing I move up the PERFMON_VECTOR to 0xf1, it will be in the
> > top priority class (15) with the following vectors:
> > 
> > Today, the PMU interrupt handler runs with interrupts disabled,
> > as such the situation would not be different. In fact, we could
> > skip masking/unmasking because we would be in the top class +
> > TPR masking done by the kernel on interrupt.
> > 
> > Anybody, has a problem with this?
> 
> If your goal is to be able to sample timer interrupt with PMU, you
> won't get it even after you move PMU vector one class up.  Because
> just like PMU vector, timer interrupt handler is run with interrupt
> off all the through.
> 
Are you talking about timer_interrupt()? Where is the masking happening
exactly?

-- 

-Stephane

RE: PMU and timer interrupts

[Chen, Kenneth W]

Stephane Eranian wrote on Friday, February 10, 2006 1:23 PM
> > If your goal is to be able to sample timer interrupt with PMU, you
> > won't get it even after you move PMU vector one class up.  Because
> > just like PMU vector, timer interrupt handler is run with interrupt
> > off all the through.
> > 
> Are you talking about timer_interrupt()?

Yes.


> Where is the masking happening exactly?

By hardware, upon an interrupt, both psr.i and psr.ic bit are off when
the control goes into the ivt:interrupt().  In there, pt_regs is saved
and only psr.ic bit is turned on.  It eventually calls down to
handle_IRQ_event(). Since SA_INTERRUPT flag is on for timer interrupt,
kernel doesn't enable irq when invoking the handler.

In fact, most of the interrupts having SA_INTERRUPT flag set.  So the
blind spots you worried about for PMU is a lot wider.  For example, on
my system, serial, e1000, mpt fusion, qla2300 interrupts all have that
flag set. Looks like kernel executes these handlers with irq disabled
all the way through.

- Ken

Re: PMU and timer interrupts

[Stephane Eranian]

Ken,

On Fri, Feb 10, 2006 at 03:04:57PM -0800, Chen, Kenneth W wrote:
> 
> > Where is the masking happening exactly?
> 
> By hardware, upon an interrupt, both psr.i and psr.ic bit are off when
> the control goes into the ivt:interrupt().  In there, pt_regs is saved
> and only psr.ic bit is turned on.  It eventually calls down to
> handle_IRQ_event(). Since SA_INTERRUPT flag is on for timer interrupt,
> kernel doesn't enable irq when invoking the handler.
> 
> In fact, most of the interrupts having SA_INTERRUPT flag set.  So the
> blind spots you worried about for PMU is a lot wider.  For example, on
> my system, serial, e1000, mpt fusion, qla2300 interrupts all have that
> flag set. Looks like kernel executes these handlers with irq disabled
> all the way through.
> 
Yes, it is coded that way. The definition of SA_INTERRUPT  is far from
clear because it is obsolete when used by applications for signal
handlers. Yet the flag is overloaded for irq handlers, apparently
where it takes a completely different meaning.

Does it make sense to have the handlers run while interrupts mask 
during their entire execution? 

I need to correct the PMU interrupt handler because it is using this
flag while also explcitely masking interrupts again.

You are right that this creates very large blind spots when monitoring,
at least with the regular interrupt-based approach.

By construction, I would not expect those in the top class (15)
to have this flag set.

-- 
-Stephane

Re: PMU and timer interrupts

[Jes Sorensen]

>>>>> "Ken" = Chen, Kenneth W <kenneth.w.chen@intel.com> writes:

Ken> By hardware, upon an interrupt, both psr.i and psr.ic bit are off
Ken> when the control goes into the ivt:interrupt().  In there,
Ken> pt_regs is saved and only psr.ic bit is turned on.  It eventually
Ken> calls down to handle_IRQ_event(). Since SA_INTERRUPT flag is on
Ken> for timer interrupt, kernel doesn't enable irq when invoking the
Ken> handler.

Ken> In fact, most of the interrupts having SA_INTERRUPT flag set.  So
Ken> the blind spots you worried about for PMU is a lot wider.  For
Ken> example, on my system, serial, e1000, mpt fusion, qla2300
Ken> interrupts all have that flag set. Looks like kernel executes
Ken> these handlers with irq disabled all the way through.

Ken,

I wonder if that isn't mostly bugs on the driver authors side?
SA_INTERRUPT was really introduced to handle UART interrupts, whereas
I think  all the other drivers you mention ought to be able to handle
it correctly.

Cheers,
Jes

Re: PMU and timer interrupts

[Stephane Eranian]

Jes,

On Fri, Feb 10, 2006 at 06:26:34PM -0500, Jes Sorensen wrote:
> >>>>> "Ken" = Chen, Kenneth W <kenneth.w.chen@intel.com> writes:
> 
> Ken> By hardware, upon an interrupt, both psr.i and psr.ic bit are off
> Ken> when the control goes into the ivt:interrupt().  In there,
> Ken> pt_regs is saved and only psr.ic bit is turned on.  It eventually
> Ken> calls down to handle_IRQ_event(). Since SA_INTERRUPT flag is on
> Ken> for timer interrupt, kernel doesn't enable irq when invoking the
> Ken> handler.
> 
> Ken> In fact, most of the interrupts having SA_INTERRUPT flag set.  So
> Ken> the blind spots you worried about for PMU is a lot wider.  For
> Ken> example, on my system, serial, e1000, mpt fusion, qla2300
> Ken> interrupts all have that flag set. Looks like kernel executes
> Ken> these handlers with irq disabled all the way through.
> 
> Ken,
> 
> I wonder if that isn't mostly bugs on the driver authors side?
> SA_INTERRUPT was really introduced to handle UART interrupts, whereas
> I think  all the other drivers you mention ought to be able to handle
> it correctly.
> 

Who would know for sure wether interrupt masking is really needed 
on those drivers?

-- 
-Stephane

Re: PMU and timer interrupts

[Jes Sorensen]

Stephane Eranian wrote:
> Jes,
>>I wonder if that isn't mostly bugs on the driver authors side?
>>SA_INTERRUPT was really introduced to handle UART interrupts, whereas
>>I think  all the other drivers you mention ought to be able to handle
>>it correctly.
> 
> Who would know for sure wether interrupt masking is really needed 
> on those drivers?

Hi Stephane,

I would start with the driver maintainers and maybe CC the subsystem
list, like linux-scsi.

I know the qla2xxx driver is very similar to the qla1280 driver which
I maintain, and that no longer uses SA_INTERRUPT. I am guessing the
qla2xxx should be able to do so as well, but check with
andrew.vasquez@qlogic.com

Cheers,
Jes

Re: PMU and timer interrupts

[Stephane Eranian]

Ken,

On Fri, Feb 10, 2006 at 03:04:57PM -0800, Chen, Kenneth W wrote:
> In fact, most of the interrupts having SA_INTERRUPT flag set.  So the
> blind spots you worried about for PMU is a lot wider.  For example, on
> my system, serial, e1000, mpt fusion, qla2300 interrupts all have that
> flag set. Looks like kernel executes these handlers with irq disabled
> all the way through.
> 
Looking at my 2.6.16-rc1, it appears that neither e1000 nor mpt fusion
are using SA_INTERRUPT anymore. The qla2xxx still does, though.

-- 
-Stephane

Re: PMU and timer interrupts

[Stephane Eranian]

On Mon, Feb 13, 2006 at 12:13:06PM -0800, Chen, Kenneth W wrote:
> Stephane Eranian wrote on Monday, February 13, 2006 12:02 PM
> > On Fri, Feb 10, 2006 at 03:04:57PM -0800, Chen, Kenneth W wrote:
> > > In fact, most of the interrupts having SA_INTERRUPT flag set.  So the
> > > blind spots you worried about for PMU is a lot wider.  For example, on
> > > my system, serial, e1000, mpt fusion, qla2300 interrupts all have that
> > > flag set. Looks like kernel executes these handlers with irq disabled
> > > all the way through.
> > > 
> > Looking at my 2.6.16-rc1, it appears that neither e1000 nor mpt fusion
> > are using SA_INTERRUPT anymore. The qla2xxx still does, though.
> 
> Yeah, I think you are correct.

Jes seems to believe than this is more historical than anything for qla2xxx.
Now what about the timer interrupt, why does it really need SA_INTERRUPT especially
given that it is so high priority and with the TPR mechanism. IT seems to be
a protection against MCA, IPI, IPI_RESCHEDULE based on the current layout.


-- 

-Stephane

RE: PMU and timer interrupts

[Chen, Kenneth W]

Stephane Eranian wrote on Monday, February 13, 2006 12:02 PM
> On Fri, Feb 10, 2006 at 03:04:57PM -0800, Chen, Kenneth W wrote:
> > In fact, most of the interrupts having SA_INTERRUPT flag set.  So the
> > blind spots you worried about for PMU is a lot wider.  For example, on
> > my system, serial, e1000, mpt fusion, qla2300 interrupts all have that
> > flag set. Looks like kernel executes these handlers with irq disabled
> > all the way through.
> > 
> Looking at my 2.6.16-rc1, it appears that neither e1000 nor mpt fusion
> are using SA_INTERRUPT anymore. The qla2xxx still does, though.

Yeah, I think you are correct.

- Ken

RE: PMU and timer interrupts

[Chen, Kenneth W]

Stephane Eranian wrote on Monday, February 13, 2006 12:13 PM
> Jes seems to believe than this is more historical than anything for qla2xxx.
> Now what about the timer interrupt, why does it really need SA_INTERRUPT especially
> given that it is so high priority and with the TPR mechanism. IT seems to be
> a protection against MCA, IPI, IPI_RESCHEDULE based on the current layout.

scheduler_tick function expects irq off because it calls load_balance and
will muck around with runqueue list.  Won't that race with resched IPI if
irq is not masked?

- Ken

Re: PMU and timer interrupts

[Keith Owens]

Stephane Eranian (on Mon, 13 Feb 2006 12:12:39 -0800) wrote:
>Jes seems to believe than this is more historical than anything for qla2xxx.
>Now what about the timer interrupt, why does it really need SA_INTERRUPT especially
>given that it is so high priority and with the TPR mechanism. IT seems to be
>a protection against MCA, IPI, IPI_RESCHEDULE based on the current layout.

Nit pick, the time interrupt will hold off any MCA slave rendezvous,
but not the MCA monarch event.  The latter is not masked by the kernel,
except where it is already processing an MCA monarch or INIT
monarch/slave event.

RE: PMU and timer interrupts

[Chen, Kenneth W]

Chen, Kenneth W wrote on Monday, February 13, 2006 12:32 PM
> Stephane Eranian wrote on Monday, February 13, 2006 12:13 PM
> > Jes seems to believe than this is more historical than anything for qla2xxx.
> > Now what about the timer interrupt, why does it really need SA_INTERRUPT especially
> > given that it is so high priority and with the TPR mechanism. IT seems to be
> > a protection against MCA, IPI, IPI_RESCHEDULE based on the current layout.
> 
> scheduler_tick function expects irq off because it calls load_balance and
> will muck around with runqueue list.  Won't that race with resched IPI if
> irq is not masked?

NO! Ken needs more coffee Monday morning ....

I forgot that we don't check TIF_ flag when we are still on kernel stack.
So resched IPI should be safe w.r.t timer interrupt because schedule is
called after timer_interrupt finishes.

- Ken

Re: PMU and timer interrupts

[Stephane Eranian]

Ken,

On Mon, Feb 13, 2006 at 02:49:57PM -0800, Chen, Kenneth W wrote:
> Chen, Kenneth W wrote on Monday, February 13, 2006 12:32 PM
> > Stephane Eranian wrote on Monday, February 13, 2006 12:13 PM
> > > Jes seems to believe than this is more historical than anything for qla2xxx.
> > > Now what about the timer interrupt, why does it really need SA_INTERRUPT especially
> > > given that it is so high priority and with the TPR mechanism. IT seems to be
> > > a protection against MCA, IPI, IPI_RESCHEDULE based on the current layout.
> > 
> > scheduler_tick function expects irq off because it calls load_balance and
> > will muck around with runqueue list.  Won't that race with resched IPI if
> > irq is not masked?
> 
> NO! Ken needs more coffee Monday morning ....
> 
> I forgot that we don't check TIF_ flag when we are still on kernel stack.
> So resched IPI should be safe w.r.t timer interrupt because schedule is
> called after timer_interrupt finishes.
> 
Thanks for checking on this. That leaves us with regular IPI which is
can be quite vast and MCA.

-- 
-Stephane