[GIT PULL] oprofile fixes for v2.6.37

[GIT PULL] oprofile fixes for v2.6.37

[Robert Richter]

Ingo,

please pull oprofile fixes v2.6.37:

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git urgent

Thanks,

-Robert


The following changes since commit e25804a0327dad954f7d43803178fdef2fd35b4e:

  Merge branch 'perf/core' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 into perf/urgent (2010-10-27 08:25:15 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git for-next

Santosh Shilimkar (1):
      oprofile: Fix the hang while taking the cpu offline

Tejun Heo (1):
      oprofile: Remove deprecated use of flush_scheduled_work()

 drivers/oprofile/buffer_sync.c |    2 +-
 drivers/oprofile/cpu_buffer.c  |   10 +++++++---
 drivers/oprofile/cpu_buffer.h  |    1 +
 drivers/oprofile/timer_int.c   |   13 +++++++++++++
 4 files changed, 22 insertions(+), 4 deletions(-)

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [GIT PULL] oprofile fixes for v2.6.37

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> Ingo,
> 
> please pull oprofile fixes v2.6.37:
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git urgent
> 
> Thanks,
> 
> -Robert
> 
> 
> The following changes since commit e25804a0327dad954f7d43803178fdef2fd35b4e:
> 
>   Merge branch 'perf/core' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 into perf/urgent (2010-10-27 08:25:15 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git for-next
> 
> Santosh Shilimkar (1):
>       oprofile: Fix the hang while taking the cpu offline
> 
> Tejun Heo (1):
>       oprofile: Remove deprecated use of flush_scheduled_work()
> 
>  drivers/oprofile/buffer_sync.c |    2 +-
>  drivers/oprofile/cpu_buffer.c  |   10 +++++++---
>  drivers/oprofile/cpu_buffer.h  |    1 +
>  drivers/oprofile/timer_int.c   |   13 +++++++++++++
>  4 files changed, 22 insertions(+), 4 deletions(-)

Pulled, thanks Robert!

	Ingo

[GIT PULL] oprofile fixes for v2.6.37

[Robert Richter]

Ingo,

please pull oprofile fixes for v2.6.37 (tip/perf/urgent):

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git urgent

(uploaded but not yet sync'ed until now)

Thanks,

-Robert


The following changes since commit e1e359273576ee8fe27021356b064c772ed29af3:

  ring_buffer: Off-by-one and duplicate events in ring_buffer_read_page (2010-12-23 12:09:30 -0500)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git urgent

Andrew Morton (1):
      arch/x86/oprofile/op_model_amd.c: perform initialisation on a single CPU

 arch/x86/oprofile/op_model_amd.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)


-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [GIT PULL] oprofile fixes for v2.6.37

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> Ingo,
> 
> please pull oprofile fixes for v2.6.37 (tip/perf/urgent):
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git urgent
> 
> (uploaded but not yet sync'ed until now)
> 
> Thanks,
> 
> -Robert
> 
> 
> The following changes since commit e1e359273576ee8fe27021356b064c772ed29af3:
> 
>   ring_buffer: Off-by-one and duplicate events in ring_buffer_read_page (2010-12-23 12:09:30 -0500)
> 
> are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git urgent
> 
> Andrew Morton (1):
>       arch/x86/oprofile/op_model_amd.c: perform initialisation on a single CPU
> 
>  arch/x86/oprofile/op_model_amd.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)

Hm, i'm not sure this fix is correct:

 static int op_amd_init(struct oprofile_operations *ops)
 {
+       /*
+        * init_ibs() preforms implictly cpu-local operations, so pin this
+        * thread to its current CPU
+        */
+       preempt_disable();
        init_ibs();
+       preempt_enable();

If init_ibs() is indeed CPU local, then it needs to be called on all CPUs.
Does that happen and if not why not? AFAICS it's only called on one CPU.

Thanks,

	Ingo

Re: [GIT PULL] oprofile fixes for v2.6.37

[Robert Richter]

On 29.12.10 11:37:43, Ingo Molnar wrote:

> Hm, i'm not sure this fix is correct:
> 
>  static int op_amd_init(struct oprofile_operations *ops)
>  {
> +       /*
> +        * init_ibs() preforms implictly cpu-local operations, so pin this
> +        * thread to its current CPU
> +        */
> +       preempt_disable();
>         init_ibs();
> +       preempt_enable();
> 
> If init_ibs() is indeed CPU local, then it needs to be called on all CPUs.
> Does that happen and if not why not? AFAICS it's only called on one CPU.

It is correct to run init_ibs() only on one cpu.  It only checks the
ibs capabilities and sets up pci devices (if necessary). It runs only
on one cpu but operates with the local APIC and some MSRs, thus it is
better to disable preemption.

The cpu setup itself runs in op_amd_setup_ctrs() which is per cpu.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [GIT PULL] oprofile fixes for v2.6.37

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> On 29.12.10 11:37:43, Ingo Molnar wrote:
> 
> > Hm, i'm not sure this fix is correct:
> > 
> >  static int op_amd_init(struct oprofile_operations *ops)
> >  {
> > +       /*
> > +        * init_ibs() preforms implictly cpu-local operations, so pin this
> > +        * thread to its current CPU
> > +        */
> > +       preempt_disable();
> >         init_ibs();
> > +       preempt_enable();
> > 
> > If init_ibs() is indeed CPU local, then it needs to be called on all CPUs. Does 
> > that happen and if not why not? AFAICS it's only called on one CPU.
> 
> It is correct to run init_ibs() only on one cpu.  It only checks the ibs 
> capabilities and sets up pci devices (if necessary). It runs only on one cpu but 
> operates with the local APIC and some MSRs, thus it is better to disable 
> preemption.

Ok, but in that case the prempt_disable()/enable() should be put into init_ibs(), 
not be open-coded at the caller like that.

The comment about its cpu-localness could move to init_ibs() as well.

Thanks,

	Ingo