* [PATCH v1 2/2] x86, perf,cqm: handle CPU hotplug
@ 2015-06-18 18:49 Kanaka Juvva
2015-06-18 19:47 ` Thomas Gleixner
0 siblings, 1 reply; 5+ messages in thread
From: Kanaka Juvva @ 2015-06-18 18:49 UTC (permalink / raw)
To: kanaka.d.juvva, glenn.p.williamson, matt.fleming, will.auld, andi,
ananth.s.narayan, linux-kernel, andrew.j.herdrich, tony.luck,
peterz, tglx, x86, mingo, hpa, luto, dvlasenk, bp, dave.hansen,
peter.p.waskiewicz.jr, imammedo, bp, ross.zwisler, jacob.w.shin,
dirk.j.brandewie, vikas.shivappa, edwin.verplanke,
tomasz.kantecki
Added lock in event reader function. The cqm_pick_event_reader() function
accesses cqm_cpumask and it is critical section between this and
cqm_stable().
This situation is true when a CPU is hotplugged. Mutex is used to protect
the critical section.
Signed-off-by: Kanaka Juvva <kanaka.d.juvva@linux.intel.com>
---
arch/x86/kernel/cpu/perf_event_intel_cqm.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/perf_event_intel_cqm.c b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
index 1880761..e17e37f 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_cqm.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
@@ -1239,12 +1239,15 @@ static inline void cqm_pick_event_reader(int cpu)
int phys_id = topology_physical_package_id(cpu);
int i;
+ mutex_lock(&cache_mutex);
for_each_cpu(i, &cqm_cpumask) {
if (phys_id == topology_physical_package_id(i))
- return; /* already got reader for this socket */
+ goto out; /* already got reader for this socket */
}
cpumask_set_cpu(cpu, &cqm_cpumask);
+out:
+ mutex_unlock(&cache_mutex);
}
static void intel_cqm_cpu_prepare(unsigned int cpu)
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v1 2/2] x86, perf,cqm: handle CPU hotplug
2015-06-18 18:49 [PATCH v1 2/2] x86, perf,cqm: handle CPU hotplug Kanaka Juvva
@ 2015-06-18 19:47 ` Thomas Gleixner
2015-06-18 20:00 ` Dave Hansen
2015-06-23 20:32 ` Vikas Shivappa
0 siblings, 2 replies; 5+ messages in thread
From: Thomas Gleixner @ 2015-06-18 19:47 UTC (permalink / raw)
To: Kanaka Juvva
Cc: kanaka.d.juvva, glenn.p.williamson, matt.fleming, will.auld, andi,
ananth.s.narayan, linux-kernel, andrew.j.herdrich, tony.luck,
peterz, x86, mingo, hpa, luto, dvlasenk, bp, dave.hansen,
peter.p.waskiewicz.jr, imammedo, bp, ross.zwisler, jacob.w.shin,
dirk.j.brandewie, vikas.shivappa, edwin.verplanke,
tomasz.kantecki
On Thu, 18 Jun 2015, Kanaka Juvva wrote:
> Added lock in event reader function. The cqm_pick_event_reader() function
> accesses cqm_cpumask and it is critical section between this and
> cqm_stable().
>
> This situation is true when a CPU is hotplugged. Mutex is used to protect
> the critical section.
>
> Signed-off-by: Kanaka Juvva <kanaka.d.juvva@linux.intel.com>
> ---
> arch/x86/kernel/cpu/perf_event_intel_cqm.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_cqm.c b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> index 1880761..e17e37f 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> @@ -1239,12 +1239,15 @@ static inline void cqm_pick_event_reader(int cpu)
> int phys_id = topology_physical_package_id(cpu);
> int i;
>
> + mutex_lock(&cache_mutex);
I already explained it to Vikas. You CANNOT take a mutex in that code
path as it runs with interrupts disabled on a CPU which cannot
schedule.
Sigh.
tglx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] x86, perf,cqm: handle CPU hotplug
2015-06-18 19:47 ` Thomas Gleixner
@ 2015-06-18 20:00 ` Dave Hansen
2015-06-18 20:09 ` Thomas Gleixner
2015-06-23 20:32 ` Vikas Shivappa
1 sibling, 1 reply; 5+ messages in thread
From: Dave Hansen @ 2015-06-18 20:00 UTC (permalink / raw)
To: Thomas Gleixner, Kanaka Juvva
Cc: kanaka.d.juvva, glenn.p.williamson, matt.fleming, will.auld, andi,
ananth.s.narayan, linux-kernel, andrew.j.herdrich, tony.luck,
peterz, x86, mingo, hpa, luto, dvlasenk, bp,
peter.p.waskiewicz.jr, imammedo, bp, ross.zwisler, jacob.w.shin,
dirk.j.brandewie, vikas.shivappa, edwin.verplanke,
tomasz.kantecki
On 06/18/2015 12:47 PM, Thomas Gleixner wrote:
>> > @@ -1239,12 +1239,15 @@ static inline void cqm_pick_event_reader(int cpu)
>> > int phys_id = topology_physical_package_id(cpu);
>> > int i;
>> >
>> > + mutex_lock(&cache_mutex);
> I already explained it to Vikas. You CANNOT take a mutex in that code
> path as it runs with interrupts disabled on a CPU which cannot
> schedule.
How did lockdep not blow up and scream about this?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] x86, perf,cqm: handle CPU hotplug
2015-06-18 20:00 ` Dave Hansen
@ 2015-06-18 20:09 ` Thomas Gleixner
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2015-06-18 20:09 UTC (permalink / raw)
To: Dave Hansen
Cc: Kanaka Juvva, glenn.p.williamson, matt.fleming, will.auld,
Andi Kleen, ananth.s.narayan, LKML, andrew.j.herdrich, Tony Luck,
Peter Zijlstra, x86, mingo, H. Peter Anvin, luto, dvlasenk, bp,
peter.p.waskiewicz.jr, imammedo, bp, ross.zwisler, jacob.w.shin,
dirk.j.brandewie, vikas.shivappa, edwin.verplanke,
tomasz.kantecki
On Thu, 18 Jun 2015, Dave Hansen wrote:
> On 06/18/2015 12:47 PM, Thomas Gleixner wrote:
> >> > @@ -1239,12 +1239,15 @@ static inline void cqm_pick_event_reader(int cpu)
> >> > int phys_id = topology_physical_package_id(cpu);
> >> > int i;
> >> >
> >> > + mutex_lock(&cache_mutex);
> > I already explained it to Vikas. You CANNOT take a mutex in that code
> > path as it runs with interrupts disabled on a CPU which cannot
> > schedule.
>
> How did lockdep not blow up and scream about this?
Dunno. I did not test that stuff.
The might_sleep() in mutex_lock() should catch it as well, IF that
code was ever executed AFTER boot on a running system.
Thanks,
tglx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] x86, perf,cqm: handle CPU hotplug
2015-06-18 19:47 ` Thomas Gleixner
2015-06-18 20:00 ` Dave Hansen
@ 2015-06-23 20:32 ` Vikas Shivappa
1 sibling, 0 replies; 5+ messages in thread
From: Vikas Shivappa @ 2015-06-23 20:32 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kanaka Juvva, Juvva, Kanaka D, Williamson, Glenn P, Matt Fleming,
Auld, Will, andi, ananth.s.narayan, linux-kernel,
Herdrich, Andrew J, tony.luck, peterz, x86, mingo, hpa, luto,
dvlasenk, bp, dave.hansen, peter.p.waskiewicz.jr, imammedo, bp,
ross.zwisler, jacob.w.shin, dirk.j.brandewie, vikas.shivappa,
Verplanke, Edwin, tomasz.kantecki
On Thu, 18 Jun 2015, Thomas Gleixner wrote:
> On Thu, 18 Jun 2015, Kanaka Juvva wrote:
>
>> Added lock in event reader function. The cqm_pick_event_reader() function
>> accesses cqm_cpumask and it is critical section between this and
>> cqm_stable().
>>
>> This situation is true when a CPU is hotplugged. Mutex is used to protect
>> the critical section.
>>
>> Signed-off-by: Kanaka Juvva <kanaka.d.juvva@linux.intel.com>
>> ---
>> arch/x86/kernel/cpu/perf_event_intel_cqm.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/perf_event_intel_cqm.c b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
>> index 1880761..e17e37f 100644
>> --- a/arch/x86/kernel/cpu/perf_event_intel_cqm.c
>> +++ b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
>> @@ -1239,12 +1239,15 @@ static inline void cqm_pick_event_reader(int cpu)
>> int phys_id = topology_physical_package_id(cpu);
>> int i;
>>
>> + mutex_lock(&cache_mutex);
>
> I already explained it to Vikas. You CANNOT take a mutex in that code
> path as it runs with interrupts disabled on a CPU which cannot
> schedule.
This patch also needs to be merged with the new package mask changes that
was added.
Will merge and send the fix.
Thanks,
Vikas
>
> Sigh.
>
> tglx
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-23 20:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-18 18:49 [PATCH v1 2/2] x86, perf,cqm: handle CPU hotplug Kanaka Juvva
2015-06-18 19:47 ` Thomas Gleixner
2015-06-18 20:00 ` Dave Hansen
2015-06-18 20:09 ` Thomas Gleixner
2015-06-23 20:32 ` Vikas Shivappa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox