Hi Stephane,
Thanks for the replies.
I didn't check with Paul, but I looked into the code in schedule() and it
does call local_irq_disable(), which does use the soft interrupt disabling
mechanism.
- Corey
Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
[EMAIL PROTECTED]
"stephane eranian" <[EMAIL PROTECTED]> wrote on 04/09/2008 03:46:56
PM:
> Corey,
>
>
> On Thu, Apr 10, 2008 at 12:26 AM, Corey Ashford <[EMAIL PROTECTED]>
wrote:
> > If schedule() uses lazy interrupt disabling, the problem we are seeing
could
> > result, since the PMU interrupt does not use the lazy interrupt wrapper
on
> > POWER.
> >
> Could you confirm this with Paul?
>
> > Do you think that using the pfm irq versions of this code is too much
of a
> > performance hit?
>
> You would be masking interrupts which are already masked. I don't know
> the cost for each arch.
>
>
> >
> > I don't see any way for this to work right on POWER without this
change,
> > short of changing the wrapper used for the PMU interrupt (which we've
> > already decided was a bad idea). Of course we could ifdef this code
for
> > POWER, I suppose. Ick.
>
> No, I'd rather pay the (hopefully) small penalty but have more readable
code.
>
> >
> > In any case, the fix in perfmon_smpl.c needs to be fixed, right? The
other
> > unlock in that function is a pfm_spin_unlock_irqrestore(...)
> >
>
> I don't think the change in perfmon_smpl.c is needed. There is a reason
why
> the unlock is broken down in multiple steps. We need to have
> interrupts enabled
> for vfree() in pfm_context_free(). pfm_spin_unlock_irqrestore() does
> not guarantee
> that interrupts will be enabled, it simply restores them to the state
> they were before
> spin_lock_irqsave(). I have seen situations (I think on IA-64) where
> pfm_handle_work
> is not invoked with interrupts enabled. This part of the code is
> handling a very special
> case where the context is zombie, i.e., no more controlling
> (monitoring) process.
>
> As for perfmon_intr.c, the reason this is regular spin_lock is because
> interrupts are already
> masked up to the same priority level. The PMU interrupts should be set
> very high in the
> priority levels. On X86, all interrupts are automatically masked. I
> think that we could
> enforce all interrupts masked with irqsave() to guarantee across the
> board that we have
> the same level of intr. masking.
>
>
>
> > - Corey
> >
> >
> >
> > >
> > > The reason regular spin_lock/spin_unlock are used is because the
> > > __pfm_ctxsw() routine is expected to
> > > be run with interrupts ALREADY masked by upper level code (likely
> > > schedule()). It may be that on PPC,
> > > schedule does things differently at its tail (where switch_to()) is
> > invoked.
> > >
> > > You are trying adding a BUG_ON(!irqs_disabled()) and see if you catch
> > > something. If not then I wonder
> > > how this could be related to the lazy masking for which you had to
add
> > > the special pfm_spin_lock_irq.....
> > >
> > >
> >
> > --
> >
> > Corey Ashford
> > Software Engineer
> > IBM Linux Technology Center, Linux Toolchain
> > Beaverton, OR
> > 503-578-3507
> > [EMAIL PROTECTED]
> >
> >
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
perfmon2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel