Thanks for the reply
"stephane eranian" <[EMAIL PROTECTED]> wrote on 07/15/2008 09:52:28
AM:
Looking at the code, it appears you are right about the sread and swrite
functions, but I'm now looking at the function pfm_power6_enable_counters.
In one case, the call stack looks like this:
arch/powerpc/perfmon/perfmon_power6.c: pfm_power6_enable_counters
arch/powerpc/perfmon/perfmon.c: pfm_arch_ctxswin_thread
perfmon/perfmon_ctxsw.c: __pfm_ctxswin_thread
perfmon/perfmon_ctxsw.c: pfm_ctxsw_in
arch/powerpc/kernel/process.c: __switch_to
The only code that I can find that disables interrupts in this call chain
is in __switch_to, but the call to pfm_ctxsw_in is
not inside the interrupts-disabled section. The code is as follows:
...
if (test_tsk_thread_flag(prev, TIF_PERFMON_CTXSW))
pfm_ctxsw_out(prev, new);
if (test_tsk_thread_flag(new, TIF_PERFMON_CTXSW)))
pfm_ctxsw_in(prev, new);
local_irq_save(flags);
account_system_vtime(current););
account_process_vtime(current);;
calculate_steal_time();
/*
* We can't take a PMU exception inside _switch() since there is a
* window where the kernel stack SLB and the kernel stack are out
* of sync. Hard disable here.
*/
hard_irq_disable();
last = _switch(old_thread, new_thread);
local_irq_restore(flags);,
return last;
}
The comments in __pfm_ctxswin_thread indicate that it is expected to be run
with interrupts off. Do you agree that this POWER-specific code looks
incorrect, that those if-statements should be inside the
local_irq_save/local_irq_restore block?
Thanks for your consideration,
- Corey
Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
[EMAIL PROTECTED]
> Corey,
>
> AFAIK, PMD registers can be read at 5 different times:
>
> - on pfm_read_pmds()
> - on context switch save
> - set switch save
> - on counter overflow to record a sample
> - on unload for flush PMU state to software
>
> In all situations, interrupts are indeed disabled. So you should not need
> getcpu/putcpu.
>
>
> On Tue, Jul 15, 2008 at 6:46 PM, Corey J Ashford <[EMAIL PROTECTED]>
wrote:
> > Hi Stephane,
> >
> > I was not aware that interrupts are disabled during the call the
pmd_sread
> > call, thanks for catching that. Is that also the case for the
> > enable_counters and disable_counters calls as well?
> >
> > Regards,
> >
> > - Corey
> >
> > Corey Ashford
> > Software Engineer
> > IBM Linux Technology Center, Linux Toolchain
> > Beaverton, OR
> > 503-578-3507
> > [EMAIL PROTECTED]
> >
> >
> >
> >
> > "stephane eranian" <[EMAIL PROTECTED]>
> > 07/15/2008 03:24 AM
> > Please respond to
> > [EMAIL PROTECTED]
> >
> >
> > To
> > Corey J Ashford/Beaverton/[EMAIL PROTECTED]
> > cc
> > [email protected]
> > Subject
> > Re: [PATCH 5/5] fixes for full perfmon2 on POWER
> >
> >
> >
> >
> >
> >
> > Corey,
> >
> >
> > I don't understand this patch.
> > You say:
> > "This patch fixes a problem with accessing the array of cpu-specific
PMU
> > counters, where there was a race condition that could occur where we
> > acquire the cpu number, and then have the cpu switch out from
underneath
> > us. Using get_cpu()/put_cpu() fixes this problem because get_cpu()
> > disables preemption until put_cpu() is called again."
> >
> > What do you mean by "switch from underneath". AFAIK, pmd_sread() is
called
> > with interrupts masked. I thought, this was enough to prevent any
> > preemption?
> >
> >
> >
> > On Mon, Jul 14, 2008 at 11:50 PM, Corey Ashford <[EMAIL PROTECTED]>
> > wrote:
> >> This fixes an issue on POWER4 and POWER6 where PMU exceptions need to
be
> >> disabled when the context is in masked mode.
> >>
> >> --
> >> 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 Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
perfmon2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
