* [RESEND PATCH 2/2] perf, cpu hotplug: Use cached value of smp_processor_id()
2012-10-16 7:58 [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled Srivatsa S. Bhat
@ 2012-10-16 7:58 ` Srivatsa S. Bhat
2012-10-16 16:32 ` Paul E. McKenney
2012-10-24 9:40 ` [tip:perf/core] " tip-bot for Srivatsa S. Bhat
2012-10-16 16:31 ` [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled Paul E. McKenney
2012-10-24 9:39 ` [tip:perf/core] " tip-bot for Srivatsa S. Bhat
2 siblings, 2 replies; 9+ messages in thread
From: Srivatsa S. Bhat @ 2012-10-16 7:58 UTC (permalink / raw)
To: peterz, acme; +Cc: mingo, tglx, akpm, paulmck, srivatsa.bhat, linux-kernel
The perf_cpu_notifier() macro invokes smp_processor_id() multiple
times. Optimize it by using a local variable.
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
include/linux/perf_event.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 0647805..6bfb2faa 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -803,15 +803,16 @@ static inline void perf_event_task_tick(void) { }
do { \
static struct notifier_block fn##_nb __cpuinitdata = \
{ .notifier_call = fn, .priority = CPU_PRI_PERF }; \
+ unsigned long cpu = smp_processor_id(); \
unsigned long flags; \
fn(&fn##_nb, (unsigned long)CPU_UP_PREPARE, \
- (void *)(unsigned long)smp_processor_id()); \
+ (void *)(unsigned long)cpu); \
local_irq_save(flags); \
fn(&fn##_nb, (unsigned long)CPU_STARTING, \
- (void *)(unsigned long)smp_processor_id()); \
+ (void *)(unsigned long)cpu); \
local_irq_restore(flags); \
fn(&fn##_nb, (unsigned long)CPU_ONLINE, \
- (void *)(unsigned long)smp_processor_id()); \
+ (void *)(unsigned long)cpu); \
register_cpu_notifier(&fn##_nb); \
} while (0)
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [RESEND PATCH 2/2] perf, cpu hotplug: Use cached value of smp_processor_id()
2012-10-16 7:58 ` [RESEND PATCH 2/2] perf, cpu hotplug: Use cached value of smp_processor_id() Srivatsa S. Bhat
@ 2012-10-16 16:32 ` Paul E. McKenney
2012-10-24 9:40 ` [tip:perf/core] " tip-bot for Srivatsa S. Bhat
1 sibling, 0 replies; 9+ messages in thread
From: Paul E. McKenney @ 2012-10-16 16:32 UTC (permalink / raw)
To: Srivatsa S. Bhat; +Cc: peterz, acme, mingo, tglx, akpm, linux-kernel
On Tue, Oct 16, 2012 at 01:28:17PM +0530, Srivatsa S. Bhat wrote:
> The perf_cpu_notifier() macro invokes smp_processor_id() multiple
> times. Optimize it by using a local variable.
>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>
> include/linux/perf_event.h | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 0647805..6bfb2faa 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -803,15 +803,16 @@ static inline void perf_event_task_tick(void) { }
> do { \
> static struct notifier_block fn##_nb __cpuinitdata = \
> { .notifier_call = fn, .priority = CPU_PRI_PERF }; \
> + unsigned long cpu = smp_processor_id(); \
> unsigned long flags; \
> fn(&fn##_nb, (unsigned long)CPU_UP_PREPARE, \
> - (void *)(unsigned long)smp_processor_id()); \
> + (void *)(unsigned long)cpu); \
> local_irq_save(flags); \
> fn(&fn##_nb, (unsigned long)CPU_STARTING, \
> - (void *)(unsigned long)smp_processor_id()); \
> + (void *)(unsigned long)cpu); \
> local_irq_restore(flags); \
> fn(&fn##_nb, (unsigned long)CPU_ONLINE, \
> - (void *)(unsigned long)smp_processor_id()); \
> + (void *)(unsigned long)cpu); \
> register_cpu_notifier(&fn##_nb); \
> } while (0)
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [tip:perf/core] perf, cpu hotplug: Use cached value of smp_processor_id()
2012-10-16 7:58 ` [RESEND PATCH 2/2] perf, cpu hotplug: Use cached value of smp_processor_id() Srivatsa S. Bhat
2012-10-16 16:32 ` Paul E. McKenney
@ 2012-10-24 9:40 ` tip-bot for Srivatsa S. Bhat
1 sibling, 0 replies; 9+ messages in thread
From: tip-bot for Srivatsa S. Bhat @ 2012-10-24 9:40 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, paulmck, hpa, mingo, srivatsa.bhat, tglx
Commit-ID: c13d38e4a1fd5dd07135403c613c8091af444169
Gitweb: http://git.kernel.org/tip/c13d38e4a1fd5dd07135403c613c8091af444169
Author: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
AuthorDate: Tue, 16 Oct 2012 13:28:17 +0530
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 24 Oct 2012 10:01:59 +0200
perf, cpu hotplug: Use cached value of smp_processor_id()
The perf_cpu_notifier() macro invokes smp_processor_id()
multiple times. Optimize it by using a local variable.
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: peterz@infradead.org
Cc: acme@ghostprotocols.net
Link: http://lkml.kernel.org/r/20121016075817.3572.76733.stgit@srivatsabhat.in.ibm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/perf_event.h | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 0647805..6bfb2faa 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -803,15 +803,16 @@ static inline void perf_event_task_tick(void) { }
do { \
static struct notifier_block fn##_nb __cpuinitdata = \
{ .notifier_call = fn, .priority = CPU_PRI_PERF }; \
+ unsigned long cpu = smp_processor_id(); \
unsigned long flags; \
fn(&fn##_nb, (unsigned long)CPU_UP_PREPARE, \
- (void *)(unsigned long)smp_processor_id()); \
+ (void *)(unsigned long)cpu); \
local_irq_save(flags); \
fn(&fn##_nb, (unsigned long)CPU_STARTING, \
- (void *)(unsigned long)smp_processor_id()); \
+ (void *)(unsigned long)cpu); \
local_irq_restore(flags); \
fn(&fn##_nb, (unsigned long)CPU_ONLINE, \
- (void *)(unsigned long)smp_processor_id()); \
+ (void *)(unsigned long)cpu); \
register_cpu_notifier(&fn##_nb); \
} while (0)
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled
2012-10-16 7:58 [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled Srivatsa S. Bhat
2012-10-16 7:58 ` [RESEND PATCH 2/2] perf, cpu hotplug: Use cached value of smp_processor_id() Srivatsa S. Bhat
@ 2012-10-16 16:31 ` Paul E. McKenney
2012-10-16 16:37 ` Srivatsa S. Bhat
2012-10-24 9:39 ` [tip:perf/core] " tip-bot for Srivatsa S. Bhat
2 siblings, 1 reply; 9+ messages in thread
From: Paul E. McKenney @ 2012-10-16 16:31 UTC (permalink / raw)
To: Srivatsa S. Bhat; +Cc: peterz, acme, mingo, tglx, akpm, linux-kernel
On Tue, Oct 16, 2012 at 01:28:10PM +0530, Srivatsa S. Bhat wrote:
> The CPU_STARTING notifiers are supposed to be run with irqs disabled. But the
> perf_cpu_notifier() macro invokes them without doing that. Fix it.
Color me confused... Isn't perf_event_task_tick() invoked only
from scheduler_tick(), which always has interrupts disabled?
Or are you needing to invoke it from somewhere else?
Thanx, Paul
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> ---
>
> include/linux/perf_event.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 2e90235..0647805 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -803,10 +803,13 @@ static inline void perf_event_task_tick(void) { }
> do { \
> static struct notifier_block fn##_nb __cpuinitdata = \
> { .notifier_call = fn, .priority = CPU_PRI_PERF }; \
> + unsigned long flags; \
> fn(&fn##_nb, (unsigned long)CPU_UP_PREPARE, \
> (void *)(unsigned long)smp_processor_id()); \
> + local_irq_save(flags); \
> fn(&fn##_nb, (unsigned long)CPU_STARTING, \
> (void *)(unsigned long)smp_processor_id()); \
> + local_irq_restore(flags); \
> fn(&fn##_nb, (unsigned long)CPU_ONLINE, \
> (void *)(unsigned long)smp_processor_id()); \
> register_cpu_notifier(&fn##_nb); \
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled
2012-10-16 16:31 ` [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled Paul E. McKenney
@ 2012-10-16 16:37 ` Srivatsa S. Bhat
2012-10-16 18:34 ` Paul E. McKenney
0 siblings, 1 reply; 9+ messages in thread
From: Srivatsa S. Bhat @ 2012-10-16 16:37 UTC (permalink / raw)
To: paulmck; +Cc: peterz, acme, mingo, tglx, akpm, linux-kernel
On 10/16/2012 10:01 PM, Paul E. McKenney wrote:
> On Tue, Oct 16, 2012 at 01:28:10PM +0530, Srivatsa S. Bhat wrote:
>> The CPU_STARTING notifiers are supposed to be run with irqs disabled. But the
>> perf_cpu_notifier() macro invokes them without doing that. Fix it.
>
> Color me confused...
Hehe, I believe the context provided by diff tricked you ;-)
The function I am referring to is perf_cpu_notifier(), not
perf_event_task_tick() :-)
Regards,
Srivatsa S. Bhat
> Isn't perf_event_task_tick() invoked only
> from scheduler_tick(), which always has interrupts disabled?
>
> Or are you needing to invoke it from somewhere else?
>
> Thanx, Paul
>
>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>> ---
>>
>> include/linux/perf_event.h | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>> index 2e90235..0647805 100644
>> --- a/include/linux/perf_event.h
>> +++ b/include/linux/perf_event.h
>> @@ -803,10 +803,13 @@ static inline void perf_event_task_tick(void) { }
>> do { \
>> static struct notifier_block fn##_nb __cpuinitdata = \
>> { .notifier_call = fn, .priority = CPU_PRI_PERF }; \
>> + unsigned long flags; \
>> fn(&fn##_nb, (unsigned long)CPU_UP_PREPARE, \
>> (void *)(unsigned long)smp_processor_id()); \
>> + local_irq_save(flags); \
>> fn(&fn##_nb, (unsigned long)CPU_STARTING, \
>> (void *)(unsigned long)smp_processor_id()); \
>> + local_irq_restore(flags); \
>> fn(&fn##_nb, (unsigned long)CPU_ONLINE, \
>> (void *)(unsigned long)smp_processor_id()); \
>> register_cpu_notifier(&fn##_nb); \
>>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled
2012-10-16 16:37 ` Srivatsa S. Bhat
@ 2012-10-16 18:34 ` Paul E. McKenney
2012-10-17 4:48 ` Srivatsa S. Bhat
0 siblings, 1 reply; 9+ messages in thread
From: Paul E. McKenney @ 2012-10-16 18:34 UTC (permalink / raw)
To: Srivatsa S. Bhat; +Cc: peterz, acme, mingo, tglx, akpm, linux-kernel
On Tue, Oct 16, 2012 at 10:07:15PM +0530, Srivatsa S. Bhat wrote:
> On 10/16/2012 10:01 PM, Paul E. McKenney wrote:
> > On Tue, Oct 16, 2012 at 01:28:10PM +0530, Srivatsa S. Bhat wrote:
> >> The CPU_STARTING notifiers are supposed to be run with irqs disabled. But the
> >> perf_cpu_notifier() macro invokes them without doing that. Fix it.
> >
> > Color me confused...
>
> Hehe, I believe the context provided by diff tricked you ;-)
> The function I am referring to is perf_cpu_notifier(), not
> perf_event_task_tick() :-)
It did indeed fool me! And indeed, checking the code did indeed show
that the name of interest if perf_cpu_notifier().
When using the correct name, I do indeed find places where it is
called with interrupts enabled. So...
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Regards,
> Srivatsa S. Bhat
>
> > Isn't perf_event_task_tick() invoked only
> > from scheduler_tick(), which always has interrupts disabled?
> >
> > Or are you needing to invoke it from somewhere else?
> >
> > Thanx, Paul
> >
> >> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> >> ---
> >>
> >> include/linux/perf_event.h | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> >> index 2e90235..0647805 100644
> >> --- a/include/linux/perf_event.h
> >> +++ b/include/linux/perf_event.h
> >> @@ -803,10 +803,13 @@ static inline void perf_event_task_tick(void) { }
> >> do { \
> >> static struct notifier_block fn##_nb __cpuinitdata = \
> >> { .notifier_call = fn, .priority = CPU_PRI_PERF }; \
> >> + unsigned long flags; \
> >> fn(&fn##_nb, (unsigned long)CPU_UP_PREPARE, \
> >> (void *)(unsigned long)smp_processor_id()); \
> >> + local_irq_save(flags); \
> >> fn(&fn##_nb, (unsigned long)CPU_STARTING, \
> >> (void *)(unsigned long)smp_processor_id()); \
> >> + local_irq_restore(flags); \
> >> fn(&fn##_nb, (unsigned long)CPU_ONLINE, \
> >> (void *)(unsigned long)smp_processor_id()); \
> >> register_cpu_notifier(&fn##_nb); \
> >>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled
2012-10-16 18:34 ` Paul E. McKenney
@ 2012-10-17 4:48 ` Srivatsa S. Bhat
0 siblings, 0 replies; 9+ messages in thread
From: Srivatsa S. Bhat @ 2012-10-17 4:48 UTC (permalink / raw)
To: paulmck; +Cc: peterz, acme, mingo, tglx, akpm, linux-kernel
On 10/17/2012 12:04 AM, Paul E. McKenney wrote:
> On Tue, Oct 16, 2012 at 10:07:15PM +0530, Srivatsa S. Bhat wrote:
>> On 10/16/2012 10:01 PM, Paul E. McKenney wrote:
>>> On Tue, Oct 16, 2012 at 01:28:10PM +0530, Srivatsa S. Bhat wrote:
>>>> The CPU_STARTING notifiers are supposed to be run with irqs disabled. But the
>>>> perf_cpu_notifier() macro invokes them without doing that. Fix it.
>>>
>>> Color me confused...
>>
>> Hehe, I believe the context provided by diff tricked you ;-)
>> The function I am referring to is perf_cpu_notifier(), not
>> perf_event_task_tick() :-)
>
> It did indeed fool me! And indeed, checking the code did indeed show
> that the name of interest if perf_cpu_notifier().
>
> When using the correct name, I do indeed find places where it is
> called with interrupts enabled. So...
>
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
Great! Thank you :)
Regards,
Srivatsa S. Bhat
>>
>>> Isn't perf_event_task_tick() invoked only
>>> from scheduler_tick(), which always has interrupts disabled?
>>>
>>> Or are you needing to invoke it from somewhere else?
>>>
>>> Thanx, Paul
>>>
>>>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>>>> ---
>>>>
>>>> include/linux/perf_event.h | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>>>> index 2e90235..0647805 100644
>>>> --- a/include/linux/perf_event.h
>>>> +++ b/include/linux/perf_event.h
>>>> @@ -803,10 +803,13 @@ static inline void perf_event_task_tick(void) { }
>>>> do { \
>>>> static struct notifier_block fn##_nb __cpuinitdata = \
>>>> { .notifier_call = fn, .priority = CPU_PRI_PERF }; \
>>>> + unsigned long flags; \
>>>> fn(&fn##_nb, (unsigned long)CPU_UP_PREPARE, \
>>>> (void *)(unsigned long)smp_processor_id()); \
>>>> + local_irq_save(flags); \
>>>> fn(&fn##_nb, (unsigned long)CPU_STARTING, \
>>>> (void *)(unsigned long)smp_processor_id()); \
>>>> + local_irq_restore(flags); \
>>>> fn(&fn##_nb, (unsigned long)CPU_ONLINE, \
>>>> (void *)(unsigned long)smp_processor_id()); \
>>>> register_cpu_notifier(&fn##_nb); \
>>>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip:perf/core] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled
2012-10-16 7:58 [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled Srivatsa S. Bhat
2012-10-16 7:58 ` [RESEND PATCH 2/2] perf, cpu hotplug: Use cached value of smp_processor_id() Srivatsa S. Bhat
2012-10-16 16:31 ` [RESEND PATCH 1/2] perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled Paul E. McKenney
@ 2012-10-24 9:39 ` tip-bot for Srivatsa S. Bhat
2 siblings, 0 replies; 9+ messages in thread
From: tip-bot for Srivatsa S. Bhat @ 2012-10-24 9:39 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, paulmck, hpa, mingo, srivatsa.bhat, tglx
Commit-ID: 6760bca9fd16256210f4922a3e9f067d2c7017d7
Gitweb: http://git.kernel.org/tip/6760bca9fd16256210f4922a3e9f067d2c7017d7
Author: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
AuthorDate: Tue, 16 Oct 2012 13:28:10 +0530
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 24 Oct 2012 10:01:58 +0200
perf, cpu hotplug: Run CPU_STARTING notifiers with irqs disabled
The CPU_STARTING notifiers are supposed to be run with irqs
disabled. But the perf_cpu_notifier() macro invokes them without
doing that. Fix it.
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: peterz@infradead.org
Cc: acme@ghostprotocols.net
Link: http://lkml.kernel.org/r/20121016075809.3572.47848.stgit@srivatsabhat.in.ibm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/perf_event.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 2e90235..0647805 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -803,10 +803,13 @@ static inline void perf_event_task_tick(void) { }
do { \
static struct notifier_block fn##_nb __cpuinitdata = \
{ .notifier_call = fn, .priority = CPU_PRI_PERF }; \
+ unsigned long flags; \
fn(&fn##_nb, (unsigned long)CPU_UP_PREPARE, \
(void *)(unsigned long)smp_processor_id()); \
+ local_irq_save(flags); \
fn(&fn##_nb, (unsigned long)CPU_STARTING, \
(void *)(unsigned long)smp_processor_id()); \
+ local_irq_restore(flags); \
fn(&fn##_nb, (unsigned long)CPU_ONLINE, \
(void *)(unsigned long)smp_processor_id()); \
register_cpu_notifier(&fn##_nb); \
^ permalink raw reply related [flat|nested] 9+ messages in thread