--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.
Greg Banks <[email protected]> wrote:
>
> + struct oprofile_cpu_buffer * cpu_buf = &cpu_buffer[smp_processor_id()];
oprofile is currently doing suspicious things with smp_processor_id() in
premptible reasons. Is this patch compounding things?
On Tue, 2004-11-09 at 22:05, Andrew Morton wrote:
> Greg Banks <[email protected]> wrote:
> >
> > + struct oprofile_cpu_buffer * cpu_buf = &cpu_buffer[smp_processor_id()];
>
> oprofile is currently doing suspicious things with smp_processor_id() in
> premptible reasons. Is this patch compounding things?
It's not changing the contexts where smp_processor_id() is called,
just pushing it down one level from a bunch of interrupt handlers
to the 2 oprofile sampling functions they call. If it was busted
before it's no more nor less busted now.
I presume the perceived problem is that with CONFIG_PREEMPT=y the
thread can be pre-empted onto another CPU? If it makes everyone
happier I can sprinkle a few preempt_disable()s around, but I'd
prefer to do it in a subsequent patch rather than respin this.
Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.
On Tue, Nov 09, 2004 at 10:35:48PM +1100, Greg Banks wrote:
> > oprofile is currently doing suspicious things with smp_processor_id() in
> > premptible reasons. Is this patch compounding things?
>
> It's not changing the contexts where smp_processor_id() is called,
> just pushing it down one level from a bunch of interrupt handlers
> to the 2 oprofile sampling functions they call. If it was busted
> before it's no more nor less busted now.
>
> I presume the perceived problem is that with CONFIG_PREEMPT=y the
> thread can be pre-empted onto another CPU? If it makes everyone
> happier I can sprinkle a few preempt_disable()s around, but I'd
> prefer to do it in a subsequent patch rather than respin this.
Andrew: basically the warning is false, there is no bug in this code.
we don't want to use preempt_disable(). Instead we want some way to get
a CPU ID and then carry on in pre-emptible fashion. It's only used to
index into an array, and if we get pre-empted onto another CPU it's not
a major deal.
(Yes, this breaks with CPU hotplug, but so does the rest of OProfile and
I've yet to see a sensible API for handling this, that is a ctor/dtor
style API)
regards
john