* [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
@ 2016-06-01 10:04 Borislav Petkov
2016-06-01 13:22 ` Guenter Roeck
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2016-06-01 10:04 UTC (permalink / raw)
To: linux-hwmon; +Cc: X86 ML, LKML, Rui Huang, Sherry Hurwitz, Guenter Roeck
From: Borislav Petkov <bp@suse.de>
We need to read a bunch of registers on each compute unit and possibly
on the current CPU too. Disable preemption around it.
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Rui Huang <ray.huang@amd.com>
Cc: Sherry Hurwitz <sherry.hurwitz@amd.com>
Cc: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/fam15h_power.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c
index eb97a9241d17..69bb810f528b 100644
--- a/drivers/hwmon/fam15h_power.c
+++ b/drivers/hwmon/fam15h_power.c
@@ -172,9 +172,9 @@ static void do_read_registers_on_cu(void *_data)
*/
static int read_registers(struct fam15h_power_data *data)
{
- int this_cpu, ret, cpu;
int core, this_core;
cpumask_var_t mask;
+ int ret, cpu;
ret = zalloc_cpumask_var(&mask, GFP_KERNEL);
if (!ret)
@@ -183,7 +183,6 @@ static int read_registers(struct fam15h_power_data *data)
memset(data->cu_on, 0, sizeof(int) * MAX_CUS);
get_online_cpus();
- this_cpu = smp_processor_id();
/*
* Choose the first online core of each compute unit, and then
@@ -205,10 +204,13 @@ static int read_registers(struct fam15h_power_data *data)
cpumask_set_cpu(cpumask_any(topology_sibling_cpumask(cpu)), mask);
}
- if (cpumask_test_cpu(this_cpu, mask))
+ preempt_disable();
+ smp_call_function_many(mask, do_read_registers_on_cu, data, true);
+
+ if (cpumask_test_cpu(smp_processor_id(), mask))
do_read_registers_on_cu(data);
- smp_call_function_many(mask, do_read_registers_on_cu, data, true);
+ preempt_enable();
put_online_cpus();
free_cpumask_var(mask);
--
2.7.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-01 10:04 [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers Borislav Petkov
@ 2016-06-01 13:22 ` Guenter Roeck
2016-06-01 13:41 ` Borislav Petkov
2016-06-01 15:26 ` Huang, Ray
0 siblings, 2 replies; 14+ messages in thread
From: Guenter Roeck @ 2016-06-01 13:22 UTC (permalink / raw)
To: Borislav Petkov, linux-hwmon; +Cc: X86 ML, LKML, Rui Huang, Sherry Hurwitz
On 06/01/2016 03:04 AM, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
>
> We need to read a bunch of registers on each compute unit and possibly
> on the current CPU too. Disable preemption around it.
>
An explanation would be helpful. Is this a bug fix ? I would like to get
a confirmation from someone at AMD that this is really necessary.
Thanks,
Guenter
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Rui Huang <ray.huang@amd.com>
> Cc: Sherry Hurwitz <sherry.hurwitz@amd.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/fam15h_power.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c
> index eb97a9241d17..69bb810f528b 100644
> --- a/drivers/hwmon/fam15h_power.c
> +++ b/drivers/hwmon/fam15h_power.c
> @@ -172,9 +172,9 @@ static void do_read_registers_on_cu(void *_data)
> */
> static int read_registers(struct fam15h_power_data *data)
> {
> - int this_cpu, ret, cpu;
> int core, this_core;
> cpumask_var_t mask;
> + int ret, cpu;
>
> ret = zalloc_cpumask_var(&mask, GFP_KERNEL);
> if (!ret)
> @@ -183,7 +183,6 @@ static int read_registers(struct fam15h_power_data *data)
> memset(data->cu_on, 0, sizeof(int) * MAX_CUS);
>
> get_online_cpus();
> - this_cpu = smp_processor_id();
>
> /*
> * Choose the first online core of each compute unit, and then
> @@ -205,10 +204,13 @@ static int read_registers(struct fam15h_power_data *data)
> cpumask_set_cpu(cpumask_any(topology_sibling_cpumask(cpu)), mask);
> }
>
> - if (cpumask_test_cpu(this_cpu, mask))
> + preempt_disable();
> + smp_call_function_many(mask, do_read_registers_on_cu, data, true);
> +
> + if (cpumask_test_cpu(smp_processor_id(), mask))
> do_read_registers_on_cu(data);
>
> - smp_call_function_many(mask, do_read_registers_on_cu, data, true);
> + preempt_enable();
> put_online_cpus();
>
> free_cpumask_var(mask);
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-01 13:22 ` Guenter Roeck
@ 2016-06-01 13:41 ` Borislav Petkov
2016-06-01 18:15 ` Guenter Roeck
2016-06-01 15:26 ` Huang, Ray
1 sibling, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2016-06-01 13:41 UTC (permalink / raw)
To: Guenter Roeck, Ingo Molnar
Cc: linux-hwmon, X86 ML, LKML, Rui Huang, Sherry Hurwitz
On Wed, Jun 01, 2016 at 06:22:59AM -0700, Guenter Roeck wrote:
> An explanation would be helpful. Is this a bug fix ?
That's a good point - yes, it is necessary. Both smp_processor_id()
*and* smp_call_function_many() need to be called with preemption
disabled.
It did fire the BUG thing in check_preemption_disabled() with 4.7-rc1
here without those fixes.
But, we need the other patch too -
"[PATCH] x86/cpu/AMD: Extend X86_FEATURE_TOPOEXT workaround to newer models"
because the cpumask generation in read_registers() doesn't work on those
Carrizo CPUs.
IINM, the breakage came in during this merge window so we don't have to
CC:stable but both should be sent to Linus as fixes for 4.7.
You could sync with Ingo who takes/acks what... but they could go
through tip and hwmon tree too, I don't see a hard dependency between
the two - they'd only need to be in 4.7-final.
Thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-01 13:41 ` Borislav Petkov
@ 2016-06-01 18:15 ` Guenter Roeck
2016-06-02 7:26 ` Borislav Petkov
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2016-06-01 18:15 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ingo Molnar, linux-hwmon, X86 ML, LKML, Rui Huang, Sherry Hurwitz
On Wed, Jun 01, 2016 at 03:41:26PM +0200, Borislav Petkov wrote:
> On Wed, Jun 01, 2016 at 06:22:59AM -0700, Guenter Roeck wrote:
> > An explanation would be helpful. Is this a bug fix ?
>
> That's a good point - yes, it is necessary. Both smp_processor_id()
> *and* smp_call_function_many() need to be called with preemption
> disabled.
>
> It did fire the BUG thing in check_preemption_disabled() with 4.7-rc1
> here without those fixes.
>
It would be great if you can add at least part of the BUG message as well
as a Fixes: tag into the patch description. If you had, I would not have
asked, saving both of us time ;-).
> But, we need the other patch too -
>
> "[PATCH] x86/cpu/AMD: Extend X86_FEATURE_TOPOEXT workaround to newer models"
>
> because the cpumask generation in read_registers() doesn't work on those
> Carrizo CPUs.
>
> IINM, the breakage came in during this merge window so we don't have to
> CC:stable but both should be sent to Linus as fixes for 4.7.
>
> You could sync with Ingo who takes/acks what... but they could go
> through tip and hwmon tree too, I don't see a hard dependency between
> the two - they'd only need to be in 4.7-final.
>
Sounds like separate trees should be fine then.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-01 18:15 ` Guenter Roeck
@ 2016-06-02 7:26 ` Borislav Petkov
2016-06-02 7:47 ` Thomas Gleixner
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2016-06-02 7:26 UTC (permalink / raw)
To: Guenter Roeck
Cc: Ingo Molnar, linux-hwmon, X86 ML, LKML, Rui Huang, Sherry Hurwitz
On Wed, Jun 01, 2016 at 11:15:09AM -0700, Guenter Roeck wrote:
> It would be great if you can add at least part of the BUG message as well
> as a Fixes: tag into the patch description.
How's that:
---
From: Borislav Petkov <bp@suse.de>
Date: Wed, 1 Jun 2016 11:36:13 +0200
Subject: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
We need to read a bunch of registers on each compute unit and possibly
on the current CPU too. Disable preemption around it. Otherwise, you
get:
BUG: using smp_processor_id() in preemptible [00000000] code: systemd-udevd/327
caller is read_registers+0x6a/0x110 [fam15h_power]
CPU: 3 PID: 327 Comm: systemd-udevd Not tainted 4.7.0-rc1+ #4
Hardware name: HP HP EliteBook 745 G3/807E, BIOS N73 Ver. 01.08 01/28/2016
...
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Rui Huang <ray.huang@amd.com>
Cc: Sherry Hurwitz <sherry.hurwitz@amd.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Fixes: fa7943449943 ("hwmon: (fam15h_power) Add compute unit accumulated power")
---
drivers/hwmon/fam15h_power.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c
index eb97a9241d17..69bb810f528b 100644
--- a/drivers/hwmon/fam15h_power.c
+++ b/drivers/hwmon/fam15h_power.c
@@ -172,9 +172,9 @@ static void do_read_registers_on_cu(void *_data)
*/
static int read_registers(struct fam15h_power_data *data)
{
- int this_cpu, ret, cpu;
int core, this_core;
cpumask_var_t mask;
+ int ret, cpu;
ret = zalloc_cpumask_var(&mask, GFP_KERNEL);
if (!ret)
@@ -183,7 +183,6 @@ static int read_registers(struct fam15h_power_data *data)
memset(data->cu_on, 0, sizeof(int) * MAX_CUS);
get_online_cpus();
- this_cpu = smp_processor_id();
/*
* Choose the first online core of each compute unit, and then
@@ -205,10 +204,13 @@ static int read_registers(struct fam15h_power_data *data)
cpumask_set_cpu(cpumask_any(topology_sibling_cpumask(cpu)), mask);
}
- if (cpumask_test_cpu(this_cpu, mask))
+ preempt_disable();
+ smp_call_function_many(mask, do_read_registers_on_cu, data, true);
+
+ if (cpumask_test_cpu(smp_processor_id(), mask))
do_read_registers_on_cu(data);
- smp_call_function_many(mask, do_read_registers_on_cu, data, true);
+ preempt_enable();
put_online_cpus();
free_cpumask_var(mask);
--
2.7.3
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-02 7:26 ` Borislav Petkov
@ 2016-06-02 7:47 ` Thomas Gleixner
2016-06-02 7:58 ` Borislav Petkov
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2016-06-02 7:47 UTC (permalink / raw)
To: Borislav Petkov
Cc: Guenter Roeck, Ingo Molnar, linux-hwmon, X86 ML, LKML, Rui Huang,
Sherry Hurwitz
On Thu, 2 Jun 2016, Borislav Petkov wrote:
> static int read_registers(struct fam15h_power_data *data)
> {
> - int this_cpu, ret, cpu;
> int core, this_core;
> cpumask_var_t mask;
> + int ret, cpu;
>
> ret = zalloc_cpumask_var(&mask, GFP_KERNEL);
> if (!ret)
> @@ -183,7 +183,6 @@ static int read_registers(struct fam15h_power_data *data)
> memset(data->cu_on, 0, sizeof(int) * MAX_CUS);
>
> get_online_cpus();
> - this_cpu = smp_processor_id();
>
> /*
> * Choose the first online core of each compute unit, and then
> @@ -205,10 +204,13 @@ static int read_registers(struct fam15h_power_data *data)
> cpumask_set_cpu(cpumask_any(topology_sibling_cpumask(cpu)), mask);
> }
>
> - if (cpumask_test_cpu(this_cpu, mask))
> + preempt_disable();
> + smp_call_function_many(mask, do_read_registers_on_cu, data, true);
> +
> + if (cpumask_test_cpu(smp_processor_id(), mask))
> do_read_registers_on_cu(data);
>
> - smp_call_function_many(mask, do_read_registers_on_cu, data, true);
> + preempt_enable();
> put_online_cpus();
What's wrong with using:
on_each_cpu_mask()
Which does all that magic for you?
Thanks,
tglx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-02 7:47 ` Thomas Gleixner
@ 2016-06-02 7:58 ` Borislav Petkov
2016-06-02 8:14 ` Borislav Petkov
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2016-06-02 7:58 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Guenter Roeck, Ingo Molnar, linux-hwmon, X86 ML, LKML, Rui Huang,
Sherry Hurwitz
On Thu, Jun 02, 2016 at 09:47:54AM +0200, Thomas Gleixner wrote:
> What's wrong with using:
>
> on_each_cpu_mask()
>
> Which does all that magic for you?
Ha, very nice! Let me try it.
Thanks!
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-02 7:58 ` Borislav Petkov
@ 2016-06-02 8:14 ` Borislav Petkov
2016-06-03 18:18 ` Guenter Roeck
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2016-06-02 8:14 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Guenter Roeck, Ingo Molnar, linux-hwmon, X86 ML, LKML, Rui Huang,
Sherry Hurwitz
On Thu, Jun 02, 2016 at 09:58:30AM +0200, Borislav Petkov wrote:
> Ha, very nice! Let me try it.
Yap, much better than opencoding on_each_cpu_mask() :-)
---
From: Borislav Petkov <bp@suse.de>
Date: Wed, 1 Jun 2016 11:36:13 +0200
Subject: [PATCH] hwmon: (fam15h_power) Disable preemption when reading
registers
We need to read a bunch of registers on each compute unit and possibly
on the current CPU too. Disable preemption around it. Otherwise, you
get:
BUG: using smp_processor_id() in preemptible [00000000] code: systemd-udevd/327
caller is read_registers+0x6a/0x110 [fam15h_power]
CPU: 3 PID: 327 Comm: systemd-udevd Not tainted 4.7.0-rc1+ #4
Hardware name: HP HP EliteBook 745 G3/807E, BIOS N73 Ver. 01.08 01/28/2016
...
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Rui Huang <ray.huang@amd.com>
Cc: Sherry Hurwitz <sherry.hurwitz@amd.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Fixes: fa7943449943 ("hwmon: (fam15h_power) Add compute unit accumulated power")
---
drivers/hwmon/fam15h_power.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c
index eb97a9241d17..15aa49d082c4 100644
--- a/drivers/hwmon/fam15h_power.c
+++ b/drivers/hwmon/fam15h_power.c
@@ -172,9 +172,9 @@ static void do_read_registers_on_cu(void *_data)
*/
static int read_registers(struct fam15h_power_data *data)
{
- int this_cpu, ret, cpu;
int core, this_core;
cpumask_var_t mask;
+ int ret, cpu;
ret = zalloc_cpumask_var(&mask, GFP_KERNEL);
if (!ret)
@@ -183,7 +183,6 @@ static int read_registers(struct fam15h_power_data *data)
memset(data->cu_on, 0, sizeof(int) * MAX_CUS);
get_online_cpus();
- this_cpu = smp_processor_id();
/*
* Choose the first online core of each compute unit, and then
@@ -205,12 +204,9 @@ static int read_registers(struct fam15h_power_data *data)
cpumask_set_cpu(cpumask_any(topology_sibling_cpumask(cpu)), mask);
}
- if (cpumask_test_cpu(this_cpu, mask))
- do_read_registers_on_cu(data);
+ on_each_cpu_mask(mask, do_read_registers_on_cu, data, true);
- smp_call_function_many(mask, do_read_registers_on_cu, data, true);
put_online_cpus();
-
free_cpumask_var(mask);
return 0;
--
2.7.3
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-02 8:14 ` Borislav Petkov
@ 2016-06-03 18:18 ` Guenter Roeck
2016-06-03 18:29 ` Borislav Petkov
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2016-06-03 18:18 UTC (permalink / raw)
To: Borislav Petkov
Cc: Thomas Gleixner, Ingo Molnar, linux-hwmon, X86 ML, LKML,
Rui Huang, Sherry Hurwitz
On Thu, Jun 02, 2016 at 10:14:39AM +0200, Borislav Petkov wrote:
> On Thu, Jun 02, 2016 at 09:58:30AM +0200, Borislav Petkov wrote:
> > Ha, very nice! Let me try it.
>
> Yap, much better than opencoding on_each_cpu_mask() :-)
>
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Wed, 1 Jun 2016 11:36:13 +0200
> Subject: [PATCH] hwmon: (fam15h_power) Disable preemption when reading
> registers
>
> We need to read a bunch of registers on each compute unit and possibly
> on the current CPU too. Disable preemption around it. Otherwise, you
> get:
>
> BUG: using smp_processor_id() in preemptible [00000000] code: systemd-udevd/327
> caller is read_registers+0x6a/0x110 [fam15h_power]
> CPU: 3 PID: 327 Comm: systemd-udevd Not tainted 4.7.0-rc1+ #4
> Hardware name: HP HP EliteBook 745 G3/807E, BIOS N73 Ver. 01.08 01/28/2016
> ...
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Rui Huang <ray.huang@amd.com>
> Cc: Sherry Hurwitz <sherry.hurwitz@amd.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Fixes: fa7943449943 ("hwmon: (fam15h_power) Add compute unit accumulated power")
I like this version. Applied, though it would be nice to get a Tested-by: or
Acked-by: from someone at AMD. I'll hold back until early next week before
sending it to Linus.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-03 18:18 ` Guenter Roeck
@ 2016-06-03 18:29 ` Borislav Petkov
2016-06-04 14:55 ` Guenter Roeck
0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2016-06-03 18:29 UTC (permalink / raw)
To: Guenter Roeck
Cc: Thomas Gleixner, Ingo Molnar, linux-hwmon, X86 ML, LKML,
Rui Huang, Sherry Hurwitz
On Fri, Jun 03, 2016 at 11:18:36AM -0700, Guenter Roeck wrote:
> I like this version. Applied, though it would be nice to get a
> Tested-by: or Acked-by: from someone at AMD. I'll hold back until
> early next week before sending it to Linus.
If it makes you feel any better: I have the affected hardware and am
testing on it too. I wouldnt've caught it otherwise.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-03 18:29 ` Borislav Petkov
@ 2016-06-04 14:55 ` Guenter Roeck
2016-06-06 7:58 ` Huang Rui
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2016-06-04 14:55 UTC (permalink / raw)
To: Borislav Petkov
Cc: Thomas Gleixner, Ingo Molnar, linux-hwmon, X86 ML, LKML,
Rui Huang, Sherry Hurwitz
On 06/03/2016 11:29 AM, Borislav Petkov wrote:
> On Fri, Jun 03, 2016 at 11:18:36AM -0700, Guenter Roeck wrote:
>> I like this version. Applied, though it would be nice to get a
>> Tested-by: or Acked-by: from someone at AMD. I'll hold back until
>> early next week before sending it to Linus.
>
> If it makes you feel any better: I have the affected hardware and am
> testing on it too. I wouldnt've caught it otherwise.
>
I'll send it to Linus next week unless I get an objection.
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-04 14:55 ` Guenter Roeck
@ 2016-06-06 7:58 ` Huang Rui
0 siblings, 0 replies; 14+ messages in thread
From: Huang Rui @ 2016-06-06 7:58 UTC (permalink / raw)
To: Guenter Roeck
Cc: Borislav Petkov, Thomas Gleixner, Ingo Molnar, linux-hwmon,
X86 ML, LKML, Sherry Hurwitz
On Sat, Jun 04, 2016 at 07:55:08AM -0700, Guenter Roeck wrote:
> On 06/03/2016 11:29 AM, Borislav Petkov wrote:
> >On Fri, Jun 03, 2016 at 11:18:36AM -0700, Guenter Roeck wrote:
> >>I like this version. Applied, though it would be nice to get a
> >>Tested-by: or Acked-by: from someone at AMD. I'll hold back until
> >>early next week before sending it to Linus.
> >
> >If it makes you feel any better: I have the affected hardware and am
> >testing on it too. I wouldnt've caught it otherwise.
> >
>
> I'll send it to Linus next week unless I get an objection.
>
Guenter, I already verified on AMD CZ(family 15h, model 60h) and model
70h processors.
Acked-and-tested-by: Huang Rui <ray.huang@amd.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-01 13:22 ` Guenter Roeck
2016-06-01 13:41 ` Borislav Petkov
@ 2016-06-01 15:26 ` Huang, Ray
2016-06-01 15:35 ` Huang, Ray
1 sibling, 1 reply; 14+ messages in thread
From: Huang, Ray @ 2016-06-01 15:26 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Borislav Petkov, linux-hwmon, X86 ML, LKML, Hurwitz, Sherry
> On Jun 1, 2016, at 9:23 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>
>> On 06/01/2016 03:04 AM, Borislav Petkov wrote:
>> From: Borislav Petkov <bp@suse.de>
>>
>> We need to read a bunch of registers on each compute unit and possibly
>> on the current CPU too. Disable preemption around it.
>
> An explanation would be helpful. Is this a bug fix ? I would like to get
> a confirmation from someone at AMD that this is really necessary.
>
This change looks good for me. But I am in office this week, I will test it on CZ platform next week. :-)
Thanks,
Rui
> Thanks,
> Guenter
>
>> Signed-off-by: Borislav Petkov <bp@suse.de>
>> Cc: Rui Huang <ray.huang@amd.com>
>> Cc: Sherry Hurwitz <sherry.hurwitz@amd.com>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> ---
>> drivers/hwmon/fam15h_power.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c
>> index eb97a9241d17..69bb810f528b 100644
>> --- a/drivers/hwmon/fam15h_power.c
>> +++ b/drivers/hwmon/fam15h_power.c
>> @@ -172,9 +172,9 @@ static void do_read_registers_on_cu(void *_data)
>> */
>> static int read_registers(struct fam15h_power_data *data)
>> {
>> - int this_cpu, ret, cpu;
>> int core, this_core;
>> cpumask_var_t mask;
>> + int ret, cpu;
>>
>> ret = zalloc_cpumask_var(&mask, GFP_KERNEL);
>> if (!ret)
>> @@ -183,7 +183,6 @@ static int read_registers(struct fam15h_power_data *data)
>> memset(data->cu_on, 0, sizeof(int) * MAX_CUS);
>>
>> get_online_cpus();
>> - this_cpu = smp_processor_id();
>>
>> /*
>> * Choose the first online core of each compute unit, and then
>> @@ -205,10 +204,13 @@ static int read_registers(struct fam15h_power_data *data)
>> cpumask_set_cpu(cpumask_any(topology_sibling_cpumask(cpu)), mask);
>> }
>>
>> - if (cpumask_test_cpu(this_cpu, mask))
>> + preempt_disable();
>> + smp_call_function_many(mask, do_read_registers_on_cu, data, true);
>> +
>> + if (cpumask_test_cpu(smp_processor_id(), mask))
>> do_read_registers_on_cu(data);
>>
>> - smp_call_function_many(mask, do_read_registers_on_cu, data, true);
>> + preempt_enable();
>> put_online_cpus();
>>
>> free_cpumask_var(mask);
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers
2016-06-01 15:26 ` Huang, Ray
@ 2016-06-01 15:35 ` Huang, Ray
0 siblings, 0 replies; 14+ messages in thread
From: Huang, Ray @ 2016-06-01 15:35 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Borislav Petkov, linux-hwmon, X86 ML, LKML, Hurwitz, Sherry
Sent from my iPad
> On Jun 1, 2016, at 11:27 PM, Huang, Ray <Ray.Huang@amd.com> wrote:
>
>
>
>>> On Jun 1, 2016, at 9:23 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>>
>>> On 06/01/2016 03:04 AM, Borislav Petkov wrote:
>>> From: Borislav Petkov <bp@suse.de>
>>>
>>> We need to read a bunch of registers on each compute unit and possibly
>>> on the current CPU too. Disable preemption around it.
>>
>> An explanation would be helpful. Is this a bug fix ? I would like to get
>> a confirmation from someone at AMD that this is really necessary.
>>
>
> This change looks good for me. But I am in office this week, I will test it on CZ platform next week. :-)
Sorry, fix typo. I am *not* in office this week.
Rui
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-06-06 7:53 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01 10:04 [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers Borislav Petkov
2016-06-01 13:22 ` Guenter Roeck
2016-06-01 13:41 ` Borislav Petkov
2016-06-01 18:15 ` Guenter Roeck
2016-06-02 7:26 ` Borislav Petkov
2016-06-02 7:47 ` Thomas Gleixner
2016-06-02 7:58 ` Borislav Petkov
2016-06-02 8:14 ` Borislav Petkov
2016-06-03 18:18 ` Guenter Roeck
2016-06-03 18:29 ` Borislav Petkov
2016-06-04 14:55 ` Guenter Roeck
2016-06-06 7:58 ` Huang Rui
2016-06-01 15:26 ` Huang, Ray
2016-06-01 15:35 ` Huang, Ray
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).