Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Corey Ashford]

Hello,

One of the things I've been working on is porting perfmon2 to ppc64. 
We've made a fair amount of progress on it, and support is available in 
libpfm and the perfmon2 kernel patch.

One of the things we had to work around was the "lazy interrupt 
disabling" mechanism in ppc64 Linux.  The problem was that the PMU 
exception handler (0xf00) uses the STD_EXCEPTION_PSERIES wrapper, which 
does not support lazy interrupt disabling.

This is desirable for Oprofile's use of the PMU since its handler is 
fairly simple and being able to profile interrupt protected code is 
desirable.  However, it causes problems for perfmon2, since the 
operations it performs on the thread of its PMU interrupt handler can 
cause a deadlock condition (it can end up calling spin_lock, for example).

Initially, to work around this, we created special spin_lock_irqsave and 
spin_unlock_irqrestore macros for perfmon2 which we could override for 
POWER to define them as functions which do hard disables and restores.

However, not all of the places that we need to disable interrupts were 
occurring from within the perfmon2 code.  Specifically, getting PMU 
interrupts in the middle of a schedule() call (where interrupts were 
expected to be disabled) was causing kernel hangs.

To fix this, I've gone back and removed the special spin_lock macros we 
defined in perfmon2 and have ifdef'd arch/powerpc/kernel/head_64.S file 
as follows:

/*** pSeries interrupt support ***/

         /* moved from 0xf00 */
+ #ifdef CONFIG_PERFMON
+         MASKABLE_EXCEPTION_PSERIES(., performance_monitor)
+ #else
         STD_EXCEPTION_PSERIES(., performance_monitor)
+ #endif

/*
  * An interrupt came in while soft-disabled; clear EE in SRR1,
  * clear paca->hard_enabled and return.

The downside of this change is that if someone is using Oprofile and 
they have enabled perfmon in their kernel, they will not get profile 
samples that occur in interrupt-protected regions of the kernel. 
However, they still can by configuring perfmon out of their kernel.

What do you think of this idea?  Is this something that you would object 
to when perfmon2 does go upstream to LKML?  I think we'd want to add 
some documentation somewhere that describes this side-effect of enabling 
perfmon in the ppc64 Linux kernel.  Do you have suggestions on where 
that should go?  I'm thinking perhaps in both the basic_profiling.txt 
and perfmon2.txt files in the Documentation subdirectory.

Thanks for your consideration,

- Corey

-- 
Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
cjashfor@us.ibm.com

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Olof Johansson]

On May 13, 2008, at 5:05 PM, Corey Ashford wrote:

> Hello,
>
> One of the things I've been working on is porting perfmon2 to ppc64.  
> We've made a fair amount of progress on it, and support is available  
> in libpfm and the perfmon2 kernel patch.
>
> One of the things we had to work around was the "lazy interrupt  
> disabling" mechanism in ppc64 Linux.  The problem was that the PMU  
> exception handler (0xf00) uses the STD_EXCEPTION_PSERIES wrapper,  
> which does not support lazy interrupt disabling.
>
> This is desirable for Oprofile's use of the PMU since its handler is  
> fairly simple and being able to profile interrupt protected code is  
> desirable.  However, it causes problems for perfmon2, since the  
> operations it performs on the thread of its PMU interrupt handler  
> can cause a deadlock condition (it can end up calling spin_lock, for  
> example).
>
> Initially, to work around this, we created special spin_lock_irqsave  
> and spin_unlock_irqrestore macros for perfmon2 which we could  
> override for POWER to define them as functions which do hard  
> disables and restores.
>
> However, not all of the places that we need to disable interrupts  
> were occurring from within the perfmon2 code.  Specifically, getting  
> PMU interrupts in the middle of a schedule() call (where interrupts  
> were expected to be disabled) was causing kernel hangs.
>
> To fix this, I've gone back and removed the special spin_lock macros  
> we defined in perfmon2 and have ifdef'd arch/powerpc/kernel/ 
> head_64.S file as follows:
>
> /*** pSeries interrupt support ***/
>
>        /* moved from 0xf00 */
> + #ifdef CONFIG_PERFMON
> +         MASKABLE_EXCEPTION_PSERIES(., performance_monitor)
> + #else
>        STD_EXCEPTION_PSERIES(., performance_monitor)
> + #endif
>
> /*
> * An interrupt came in while soft-disabled; clear EE in SRR1,
> * clear paca->hard_enabled and return.
>
> The downside of this change is that if someone is using Oprofile and  
> they have enabled perfmon in their kernel, they will not get profile  
> samples that occur in interrupt-protected regions of the kernel.  
> However, they still can by configuring perfmon out of their kernel.
>
> What do you think of this idea?  Is this something that you would  
> object to when perfmon2 does go upstream to LKML?  I think we'd want  
> to add some documentation somewhere that describes this side-effect  
> of enabling perfmon in the ppc64 Linux kernel.  Do you have  
> suggestions on where that should go?  I'm thinking perhaps in both  
> the basic_profiling.txt and perfmon2.txt files in the Documentation  
> subdirectory.

Since you didn't post the perfmon2 code, I'll ask instead of go look:

Do you have a single entry point from performance_monitor into  
perfmon2? If so, it's nicer to check and see if interrupts are soft  
disabled right upon entry (before taking any locks, etc), and if they  
are just return without doing more work.

PMU interrupts generally won't re-arm themselves so you'll obviously  
have to deal with that as well but I'm sure you're already aware of  
that...


-Olof

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Corey Ashford]

Corey J Ashford wrote:
> On May 13, 2008, at 5:05 PM, Corey Ashford wrote:
> 
>> Hello,
>>
>> One of the things I've been working on is porting perfmon2 to ppc64.
>> We've made a fair amount of progress on it, and support is available
>> in libpfm and the perfmon2 kernel patch.
>>
>> One of the things we had to work around was the "lazy interrupt
>> disabling" mechanism in ppc64 Linux.  The problem was that the PMU
>> exception handler (0xf00) uses the STD_EXCEPTION_PSERIES wrapper,
>> which does not support lazy interrupt disabling.
>>
>> This is desirable for Oprofile's use of the PMU since its handler is
>> fairly simple and being able to profile interrupt protected code is
>> desirable.  However, it causes problems for perfmon2, since the
>> operations it performs on the thread of its PMU interrupt handler
>> can cause a deadlock condition (it can end up calling spin_lock, for
>> example).
>>
>> Initially, to work around this, we created special spin_lock_irqsave
>> and spin_unlock_irqrestore macros for perfmon2 which we could
>> override for POWER to define them as functions which do hard
>> disables and restores.
>>
>> However, not all of the places that we need to disable interrupts
>> were occurring from within the perfmon2 code.  Specifically, getting
>> PMU interrupts in the middle of a schedule() call (where interrupts
>> were expected to be disabled) was causing kernel hangs.
>>
>> To fix this, I've gone back and removed the special spin_lock macros
>> we defined in perfmon2 and have ifdef'd arch/powerpc/kernel/
>> head_64.S file as follows:
>>
>> /*** pSeries interrupt support ***/
>>
>>        /* moved from 0xf00 */
>> + #ifdef CONFIG_PERFMON
>> +         MASKABLE_EXCEPTION_PSERIES(., performance_monitor)
>> + #else
>>        STD_EXCEPTION_PSERIES(., performance_monitor)
>> + #endif
>>
>> /*
>> * An interrupt came in while soft-disabled; clear EE in SRR1,
>> * clear paca->hard_enabled and return.
>>
>> The downside of this change is that if someone is using Oprofile and
>> they have enabled perfmon in their kernel, they will not get profile
>> samples that occur in interrupt-protected regions of the kernel.
>> However, they still can by configuring perfmon out of their kernel.
>>
>> What do you think of this idea?  Is this something that you would
>> object to when perfmon2 does go upstream to LKML?  I think we'd want
>> to add some documentation somewhere that describes this side-effect
>> of enabling perfmon in the ppc64 Linux kernel.  Do you have
>> suggestions on where that should go?  I'm thinking perhaps in both
>> the basic_profiling.txt and perfmon2.txt files in the Documentation
>> subdirectory.
> 
> Since you didn't post the perfmon2 code, I'll ask instead of go look:
> 
> Do you have a single entry point from performance_monitor into
> perfmon2? If so, it's nicer to check and see if interrupts are soft
> disabled right upon entry (before taking any locks, etc), and if they
> are just return without doing more work.
> 
> PMU interrupts generally won't re-arm themselves so you'll obviously
> have to deal with that as well but I'm sure you're already aware of
> that...
> 
> 
> -Olof

The perfmon2 code is available here: 
http://sourceforge.net/project/showfiles.php?group_id=144822

perfmon2's interrupt handler does have a single entry point.  Could I 
somehow mimic what the MASKABLE_EXCEPTION_PSERIES macro does inside of 
the perfmon2 interrupt handler?  Are there examples of this I can look at?

That would give us the best of both worlds.

Regards,

- Corey

-- 
Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
cjashfor@us.ibm.com

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Benjamin Herrenschmidt]

On Tue, 2008-05-13 at 15:26 -0700, Corey Ashford wrote:
> The perfmon2 code is available here: 
> http://sourceforge.net/project/showfiles.php?group_id=144822
> 
> perfmon2's interrupt handler does have a single entry point.  Could I 
> somehow mimic what the MASKABLE_EXCEPTION_PSERIES macro does inside
> of 
> the perfmon2 interrupt handler?  Are there examples of this I can look
> at?
> 
> That would give us the best of both worlds.

You can definitely snapshot as many data as you can, and if interrupts
are soft-disabled, just return to the caller, storing that snapshot in
some per-cpu data structure.

You can then add something to local_irq_restore() that checks whether
some perfmon2 stuff happened and does the actual storing of the data
that were previously collected.

Cheers,
Ben.

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Corey J Ashford]

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

Hi Benjamin and Olaf,

Thanks for the suggestions.

Ideally, what I'm looking for is something that mimics the operation of
MASKABLE_EXCEPTION_PSERIES.
I've been looking at the kernel code (entry_64.S, exception.h, head_64.S)
but am finding it quite complicated and hard to follow, particularly in the
area of interrupt disabling wrt the soft and hard disable logic.

My initial thought is to do something like this in the beginning of my
perfmon2 interrupt handler:

void perfmon_pmu_int_handler(struct pt_regs *regs) {

      if (get_paca()->soft_enabled == 0) {
            /* disable hardware interrupts */
            get_paca()->hard_enabled = 0;
            regs->msr &= ^MSR_EE;
            return;
      }
...
}

Does this seem like it might work?

Thanks

Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
cjashfor@us.ibm.com


Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 05/14/2008
11:46:07 PM:

>
> On Tue, 2008-05-13 at 15:26 -0700, Corey Ashford wrote:
> > The perfmon2 code is available here:
> > http://sourceforge.net/project/showfiles.php?group_id=144822
> >
> > perfmon2's interrupt handler does have a single entry point.  Could I
> > somehow mimic what the MASKABLE_EXCEPTION_PSERIES macro does inside
> > of
> > the perfmon2 interrupt handler?  Are there examples of this I can look
> > at?
> >
> > That would give us the best of both worlds.
>
> You can definitely snapshot as many data as you can, and if interrupts
> are soft-disabled, just return to the caller, storing that snapshot in
> some per-cpu data structure.
>
> You can then add something to local_irq_restore() that checks whether
> some perfmon2 stuff happened and does the actual storing of the data
> that were previously collected.
>
> Cheers,
> Ben.
>
>

[-- Attachment #2: Type: text/html, Size: 2343 bytes --]

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Corey J Ashford]

[-- Attachment #1: Type: text/plain, Size: 2477 bytes --]

Just FYI:  I just tried out the code I suggested below, and it does not
work; it results in a system hang.  I have spent some time analyzing why
this doesn't work as I expected, but so far I haven't been able to figure
it out.

Regards,

Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
cjashfor@us.ibm.com


linuxppc-dev-bounces+cjashfor=us.ibm.com@ozlabs.org wrote on 05/15/2008
12:41:55 PM:

> Hi Benjamin and Olaf,
>
> Thanks for the suggestions.
>
> Ideally, what I'm looking for is something that mimics the operation
> of MASKABLE_EXCEPTION_PSERIES.
> I've been looking at the kernel code (entry_64.S, exception.h,
> head_64.S) but am finding it quite complicated and hard to follow,
> particularly in the area of interrupt disabling wrt the soft and
> hard disable logic.
>
> My initial thought is to do something like this in the beginning of
> my perfmon2 interrupt handler:
>
> void perfmon_pmu_int_handler(struct pt_regs *regs) {
>
> if (get_paca()->soft_enabled == 0) {
> /* disable hardware interrupts */
> get_paca()->hard_enabled = 0;
> regs->msr &= ^MSR_EE;
> return;
> }
> ...
> }
>
> Does this seem like it might work?
>
> Thanks
>
> Corey Ashford
> Software Engineer
> IBM Linux Technology Center, Linux Toolchain
> Beaverton, OR
> 503-578-3507
> cjashfor@us.ibm.com
>
>
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on
> 05/14/2008 11:46:07 PM:
>
> >
> > On Tue, 2008-05-13 at 15:26 -0700, Corey Ashford wrote:
> > > The perfmon2 code is available here:
> > > http://sourceforge.net/project/showfiles.php?group_id=144822
> > >
> > > perfmon2's interrupt handler does have a single entry point.  Could I

> > > somehow mimic what the MASKABLE_EXCEPTION_PSERIES macro does inside
> > > of
> > > the perfmon2 interrupt handler?  Are there examples of this I can
look
> > > at?
> > >
> > > That would give us the best of both worlds.
> >
> > You can definitely snapshot as many data as you can, and if interrupts
> > are soft-disabled, just return to the caller, storing that snapshot in
> > some per-cpu data structure.
> >
> > You can then add something to local_irq_restore() that checks whether
> > some perfmon2 stuff happened and does the actual storing of the data
> > that were previously collected.
> >
> > Cheers,
> > Ben.
> >
> > _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

[-- Attachment #2: Type: text/html, Size: 3345 bytes --]

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Paul Mackerras]

Corey J Ashford writes:

> Ideally, what I'm looking for is something that mimics the operation of
> MASKABLE_EXCEPTION_PSERIES.
> I've been looking at the kernel code (entry_64.S, exception.h, head_64.S)
> but am finding it quite complicated and hard to follow, particularly in the
> area of interrupt disabling wrt the soft and hard disable logic.
> 
> My initial thought is to do something like this in the beginning of my
> perfmon2 interrupt handler:
> 
> void perfmon_pmu_int_handler(struct pt_regs *regs) {
> 
>       if (get_paca()->soft_enabled == 0) {
>             /* disable hardware interrupts */
>             get_paca()->hard_enabled = 0;
>             regs->msr &= ^MSR_EE;
>             return;
>       }
> ...
> }
> 
> Does this seem like it might work?

That's a start (with ~MSR_EE rather than ^MSR_EE, of course).  You
also need to set a flag in that case, and test the flag in the part of
arch/powerpc/kernel/irq.c:raw_local_irq_restore() that hard-enables
interrupts.  If the flag is set then you should call back into the
perfmon2 code at that point (and clear the flag, of course).  You
could add a field to the paca for that flag.

You probably also want to read some of the PMU's SPRs when you first
get the interrupt and save their values away, and then when you get
the call back from raw_local_irq_restore, use the saved values rather
than what's currently in the SPRs, since the saved values will be more
accurate.

Paul.

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Corey J Ashford]

[-- Attachment #1: Type: text/plain, Size: 3043 bytes --]



Paul Mackerras <paulus@samba.org> wrote on 05/15/2008 04:36:02 PM:

> Corey J Ashford writes:
>
> > Ideally, what I'm looking for is something that mimics the operation of
> > MASKABLE_EXCEPTION_PSERIES.
> > I've been looking at the kernel code (entry_64.S, exception.h,
head_64.S)
> > but am finding it quite complicated and hard to follow, particularly in
the
> > area of interrupt disabling wrt the soft and hard disable logic.
> >
> > My initial thought is to do something like this in the beginning of my
> > perfmon2 interrupt handler:
> >
> > void perfmon_pmu_int_handler(struct pt_regs *regs) {
> >
> >       if (get_paca()->soft_enabled == 0) {
> >             /* disable hardware interrupts */
> >             get_paca()->hard_enabled = 0;
> >             regs->msr &= ^MSR_EE;
> >             return;
> >       }
> > ...
> > }
> >
> > Does this seem like it might work?
>
> That's a start (with ~MSR_EE rather than ^MSR_EE, of course).  You
> also need to set a flag in that case, and test the flag in the part of
> arch/powerpc/kernel/irq.c:raw_local_irq_restore() that hard-enables
> interrupts.  If the flag is set then you should call back into the
> perfmon2 code at that point (and clear the flag, of course).  You
> could add a field to the paca for that flag.
>
> You probably also want to read some of the PMU's SPRs when you first
> get the interrupt and save their values away, and then when you get
> the call back from raw_local_irq_restore, use the saved values rather
> than what's currently in the SPRs, since the saved values will be more
> accurate.
>
> Paul.

Hi Paul,

Thanks for the feedback.  I don't believe I need a separate flag, because
the PMU interrupt (via the PMAO bit) will still be pending when interrupts
are hard enabled again, and the handler will be reentered automatically.

I'll think about the issue of caching the SPR values some more, but I don't
want to complicate things too much.  PMU event counts are not so critical
that they need to so accurately reflect the exact value at the time of the
interrupt.

I discovered through some trial and error that get_paca() doesn't work
correctly in the interrupt handler.  It appears that the value of r13 (the
PACA pointer) is not initialized.  Perhaps r13 is a scratch register used
by the compiler?

Fortunately, because the soft enable flag is available in the pt_regs
structure, and because the hard enable flag will be set to the same as the
value of regs->msr's MSR_EE flag in the restore code, I now have this code
in the interrupt handler:

void perfmon_pmu_int_handler(struct pt_regs *regs) {

      if (regs->softe == 0) {
            /* disable hardware interrupts */
            regs->msr &= ~MSR_EE;
            return;
      }
      ...
}

This code does seem to be working, but needs more testing.

Regards,

- Corey

Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
cjashfor@us.ibm.com

[-- Attachment #2: Type: text/html, Size: 3840 bytes --]

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Paul Mackerras]

Corey J Ashford writes:

> Thanks for the feedback.  I don't believe I need a separate flag, because
> the PMU interrupt (via the PMAO bit) will still be pending when interrupts
> are hard enabled again, and the handler will be reentered automatically.

If that were the case then we wouldn't have had the problem with
losing PMU interrupts that meant we had to change the PMU interrupt
handler from a MASKABLE_EXCEPTION to a STD_EXCEPTION.  This was in
commit 449d846dbcbf61bdf7d50a923e4791102168c292.

My understanding is that the PMU only requests an interrupt when PMAO
goes from 0 to 1 (i.e. it's edge-triggered).  If the CPU takes the
interrupt and then sets MSR.EE again (e.g. by returning from the
interrupt handler), and PMAO has not been reset to 0, then I don't
think the PMU requests another interrupt at that point.

> I discovered through some trial and error that get_paca() doesn't work
> correctly in the interrupt handler.  It appears that the value of r13 (the
> PACA pointer) is not initialized.  Perhaps r13 is a scratch register used
> by the compiler?

Weird.  It definitely should be correct.  Loading r13 is one of the
first things the interrupt entry code does, and r13 is not a scratch
register (it's normally the thread-local storage pointer).

If r13 is bogus then that's definitely a serious bug.

> Fortunately, because the soft enable flag is available in the pt_regs
> structure, and because the hard enable flag will be set to the same as the
> value of regs->msr's MSR_EE flag in the restore code, I now have this code
> in the interrupt handler:
> 
> void perfmon_pmu_int_handler(struct pt_regs *regs) {
> 
>       if (regs->softe == 0) {
>             /* disable hardware interrupts */
>             regs->msr &= ~MSR_EE;
>             return;
>       }
>       ...
> }
> 
> This code does seem to be working, but needs more testing.

I expect that after a little while you'll stop getting PMU
interrupts...

Paul.

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Corey J Ashford]

Paul Mackerras <paulus@samba.org> wrote on 05/15/2008 06:02:03 PM:

> Corey J Ashford writes:
> 
> > Thanks for the feedback.  I don't believe I need a separate flag, 
because
> > the PMU interrupt (via the PMAO bit) will still be pending when 
interrupts
> > are hard enabled again, and the handler will be reentered 
automatically.
> 
> If that were the case then we wouldn't have had the problem with
> losing PMU interrupts that meant we had to change the PMU interrupt
> handler from a MASKABLE_EXCEPTION to a STD_EXCEPTION.  This was in
> commit 449d846dbcbf61bdf7d50a923e4791102168c292.
> 
> My understanding is that the PMU only requests an interrupt when PMAO
> goes from 0 to 1 (i.e. it's edge-triggered).  If the CPU takes the
> interrupt and then sets MSR.EE again (e.g. by returning from the
> interrupt handler), and PMAO has not been reset to 0, then I don't
> think the PMU requests another interrupt at that point.

I think that is the case on early POWER4 and PPC970 chips where this is no 
PMAO bit.  However, from personal [bad] experience, I can say that if PMAO 
is not cleared in the interrupt handler, the exception will be taken again 
when interrupts are reenabled.  I've seen the system lockup in interrupt 
handling "loops" because it wasn't being cleared.

> 
> > I discovered through some trial and error that get_paca() doesn't work
> > correctly in the interrupt handler.  It appears that the value of r13 
(the
> > PACA pointer) is not initialized.  Perhaps r13 is a scratch register 
used
> > by the compiler?
> 
> Weird.  It definitely should be correct.  Loading r13 is one of the
> first things the interrupt entry code does, and r13 is not a scratch
> register (it's normally the thread-local storage pointer).
> 
> If r13 is bogus then that's definitely a serious bug.

My evidence for this was that I was seeing get_paca()->soft_enable and 
get_paca()->hard_enable set to zero, but regs->softe set to 1.

Looking at the code again, I see that in STD_EXCEPTION_COMMON in 
exception.h, the macro DISABLE_INTS is used, which zeros out the soft and 
hard disable flags.  So r13 is ok, and I think using the regs struct is 
the right way to go.


> 
> > Fortunately, because the soft enable flag is available in the pt_regs
> > structure, and because the hard enable flag will be set to the same as 
the
> > value of regs->msr's MSR_EE flag in the restore code, I now have this 
code
> > in the interrupt handler:
> > 
> > void perfmon_pmu_int_handler(struct pt_regs *regs) {
> > 
> >       if (regs->softe == 0) {
> >             /* disable hardware interrupts */
> >             regs->msr &= ~MSR_EE;
> >             return;
> >       }
> >       ...
> > }
> > 
> > This code does seem to be working, but needs more testing.
> 
> I expect that after a little while you'll stop getting PMU
> interrupts...
> 
> Paul.

I haven't seen that yet, but I am seeing another kernel hang with my test 
case that creates and deals with about 2000 interrupts per second.  After 
a random amount of time running it, it hangs the system.  If I restart the 
system into xmon, I see a callstack that doesn't have any perfmon code in 
it, but does have some kernel interrupt handling code.  The hang is the 
same when using MASKABLE_EXCEPTION_PSERIES and STD_EXCEPTION_PSERIES (+ 
the above fix).

I'd love to be able to run that call stack by you, if you have time to 
look at it.

Thanks,

- Corey

Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR 
503-578-3507 
cjashfor@us.ibm.com

Re: Feedback requested on switching the exception wrapper used for the PMU interrupt on ppc64

[Corey J Ashford]

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]


Paul Mackerras <paulus@samba.org> wrote on 05/15/2008 06:02:03 PM:

> Corey J Ashford writes:
>
> > Thanks for the feedback.  I don't believe I need a separate flag,
because
> > the PMU interrupt (via the PMAO bit) will still be pending when
interrupts
> > are hard enabled again, and the handler will be reentered
automatically.
>
> If that were the case then we wouldn't have had the problem with
> losing PMU interrupts that meant we had to change the PMU interrupt
> handler from a MASKABLE_EXCEPTION to a STD_EXCEPTION.  This was in
> commit 449d846dbcbf61bdf7d50a923e4791102168c292.
>
> My understanding is that the PMU only requests an interrupt when PMAO
> goes from 0 to 1 (i.e. it's edge-triggered).  If the CPU takes the
> interrupt and then sets MSR.EE again (e.g. by returning from the
> interrupt handler), and PMAO has not been reset to 0, then I don't
> think the PMU requests another interrupt at that point.
>

I went back and looked through my notes, and found that the problem with
looping in the interrupt handler I had a couple of months ago was not with
PMAO not being cleared, but because there were unused counters (PMC5 and/or
PMC6) were not zeroed out in the interrupt handler and so they continued to
count past 0x80000000, causing PMAO to become asserted on every count.
This was on POWER5+, and so the behavior may be different on other chips.
I do find it a little odd that it would set PMAO on every count past
0x80000000, instead of just on the transition from 0x7fffffff to
0x80000000.

In any case, I do need to save the fact that an interrupt occurred and to
reassert the interrupt in local_irq_restore (it appears to be the case that
raw_local_irq_restore for POWER is new in 2.6.26.  I'm working on 2.6.25 at
the moment)

Thanks for your reply, as it could have taken me a long time to find this
problem otherwise.

- Corey

Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
cjashfor@us.ibm.com

[-- Attachment #2: Type: text/html, Size: 2346 bytes --]