Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1215138 > unrolled thread

Re: [PATCH 09/15] x86, amd: add accessor for number of cores per compute unit

Started byBorislav Petkov <bp@alien8.de>
First post2015-08-28 08:50 +0200
Last post2015-09-01 18:10 +0200
Articles 18 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Borislav Petkov <bp@alien8.de> - 2015-08-28 08:50 +0200
    Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Guenter Roeck <linux@roeck-us.net> - 2015-08-28 10:10 +0200
    Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Ingo Molnar <mingo@kernel.org> - 2015-08-28 10:10 +0200
      Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Borislav Petkov <bp@alien8.de> - 2015-08-28 11:00 +0200
      Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Ingo Molnar <mingo@kernel.org> - 2015-08-29 11:20 +0200
        Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Borislav Petkov <bp@alien8.de> - 2015-08-30 18:00 +0200
          Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Peter Zijlstra <peterz@infradead.org> - 2015-08-31 10:40 +0200
            Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Guenter Roeck <linux@roeck-us.net> - 2015-08-31 15:30 +0200
              Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Peter Zijlstra <peterz@infradead.org> - 2015-08-31 15:40 +0200
                Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Guenter Roeck <linux@roeck-us.net> - 2015-08-31 16:00 +0200
                  Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Peter Zijlstra <peterz@infradead.org> - 2015-08-31 17:00 +0200
                    Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Guenter Roeck <linux@roeck-us.net> - 2015-08-31 17:20 +0200
            Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Borislav Petkov <bp@alien8.de> - 2015-08-31 18:10 +0200
              Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Guenter Roeck <linux@roeck-us.net> - 2015-08-31 18:20 +0200
                Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Peter Zijlstra <peterz@infradead.org> - 2015-08-31 22:50 +0200
                  Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Guenter Roeck <linux@roeck-us.net> - 2015-08-31 23:30 +0200
                    Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Borislav Petkov <bp@alien8.de> - 2015-09-01 18:00 +0200
                      Re: [PATCH 09/15] x86, amd: add accessor for number of cores per  compute unit Guenter Roeck <linux@roeck-us.net> - 2015-09-01 18:10 +0200

#1215138 — Re: [PATCH 09/15] x86, amd: add accessor for number of cores per compute unit

FromBorislav Petkov <bp@alien8.de>
Date2015-08-28 08:50 +0200
SubjectRe: [PATCH 09/15] x86, amd: add accessor for number of cores per compute unit
Message-ID<q2lE6-3R6-23@gated-at.bofh.it>
On Thu, Aug 27, 2015 at 04:07:40PM +0800, Huang Rui wrote:
> Add an accessor function amd_get_cores_per_cu() which returns the
> number of cores per compute unit.
> 
> In a subsequent patch, we will use this function in fam15h_power
> driver.
> 
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  arch/x86/include/asm/processor.h |  1 +
>  arch/x86/kernel/cpu/amd.c        | 19 +++++++++++++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)

Btw, this needs an ACK from a tip person if it goes through the hwmon
tree.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1215207

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-28 10:10 +0200
Message-ID<q2mTv-5Oh-3@gated-at.bofh.it>
In reply to#1215138
On 08/27/2015 11:48 PM, Borislav Petkov wrote:
> On Thu, Aug 27, 2015 at 04:07:40PM +0800, Huang Rui wrote:
>> Add an accessor function amd_get_cores_per_cu() which returns the
>> number of cores per compute unit.
>>
>> In a subsequent patch, we will use this function in fam15h_power
>> driver.
>>
>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>> ---
>>   arch/x86/include/asm/processor.h |  1 +
>>   arch/x86/kernel/cpu/amd.c        | 19 +++++++++++++++++--
>>   2 files changed, 18 insertions(+), 2 deletions(-)
>
> Btw, this needs an ACK from a tip person if it goes through the hwmon
> tree.
>
Yes, most definitely.

Guenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1215211

FromIngo Molnar <mingo@kernel.org>
Date2015-08-28 10:10 +0200
Message-ID<q2mTw-5Oh-21@gated-at.bofh.it>
In reply to#1215138
* Borislav Petkov <bp@alien8.de> wrote:

> On Thu, Aug 27, 2015 at 04:07:40PM +0800, Huang Rui wrote:
> > Add an accessor function amd_get_cores_per_cu() which returns the
> > number of cores per compute unit.
> > 
> > In a subsequent patch, we will use this function in fam15h_power
> > driver.
> > 
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> >  arch/x86/include/asm/processor.h |  1 +
> >  arch/x86/kernel/cpu/amd.c        | 19 +++++++++++++++++--
> >  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> Btw, this needs an ACK from a tip person if it goes through the hwmon
> tree.

Looks good to me in theory.

I suspect we might want to factor the 'compute unit' logic out a bit more if usage 
becomes more widespread - but right now it's hwmon drivers only,right?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1215228

FromBorislav Petkov <bp@alien8.de>
Date2015-08-28 11:00 +0200
Message-ID<q2nFU-6Jv-17@gated-at.bofh.it>
In reply to#1215211
On Fri, Aug 28, 2015 at 10:04:18AM +0200, Ingo Molnar wrote:
> Looks good to me in theory.

Thanks.

> I suspect we might want to factor the 'compute unit' logic out a bit
> more if usage becomes more widespread - but right now it's hwmon
> drivers only,right?

Yeah.

My angle is to extend stuff as we go and only when we need it.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1215737

FromIngo Molnar <mingo@kernel.org>
Date2015-08-29 11:20 +0200
Message-ID<q2KsN-5TZ-1@gated-at.bofh.it>
In reply to#1215211
* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Borislav Petkov <bp@alien8.de> wrote:
> 
> > On Thu, Aug 27, 2015 at 04:07:40PM +0800, Huang Rui wrote:
> > > Add an accessor function amd_get_cores_per_cu() which returns the
> > > number of cores per compute unit.
> > > 
> > > In a subsequent patch, we will use this function in fam15h_power
> > > driver.
> > > 
> > > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > > ---
> > >  arch/x86/include/asm/processor.h |  1 +
> > >  arch/x86/kernel/cpu/amd.c        | 19 +++++++++++++++++--
> > >  2 files changed, 18 insertions(+), 2 deletions(-)
> > 
> > Btw, this needs an ACK from a tip person if it goes through the hwmon
> > tree.
> 
> Looks good to me in theory.
> 
> I suspect we might want to factor the 'compute unit' logic out a bit more if usage 
> becomes more widespread - but right now it's hwmon drivers only,right?

So let me withdraw my ack: the much more important question that I missed first 
time around, why is this reporting feature living in hwmon, not in perf? We have 
energy reporting facilities in perf that this should be synced to.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1215938

FromBorislav Petkov <bp@alien8.de>
Date2015-08-30 18:00 +0200
Message-ID<q3dbs-53n-9@gated-at.bofh.it>
In reply to#1215737
On Sat, Aug 29, 2015 at 11:19:14AM +0200, Ingo Molnar wrote:
> So let me withdraw my ack: the much more important question that I
> missed first time around, why is this reporting feature living in
> hwmon, not in perf? We have energy reporting facilities in perf that
> this should be synced to.

Because there's already fam15h_power driver which is exactly for that.
Making it part of perf is then a question of cat-ting the same sysfs
file twice, at the beginning and at the end of the trace, which is
trivial.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216096

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-31 10:40 +0200
Message-ID<q3sNb-2pJ-5@gated-at.bofh.it>
In reply to#1215938
On Sun, Aug 30, 2015 at 05:53:22PM +0200, Borislav Petkov wrote:
> On Sat, Aug 29, 2015 at 11:19:14AM +0200, Ingo Molnar wrote:
> > So let me withdraw my ack: the much more important question that I
> > missed first time around, why is this reporting feature living in
> > hwmon, not in perf? We have energy reporting facilities in perf that
> > this should be synced to.
> 
> Because there's already fam15h_power driver which is exactly for that.
> Making it part of perf is then a question of cat-ting the same sysfs
> file twice, at the beginning and at the end of the trace, which is
> trivial.

That don't make sense.

Looking at the BKDG Fam 15h 60h-6Fh these MSRs are per compute unit.
This means you can do much finer grained measurements than system wide
-- which is all hwmon seems capable of.

Not to mention the proposed code is horrible, who in their right mind
does two rdmsrl_safe_on_cpu() back to back.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216182

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-31 15:30 +0200
Message-ID<q3xjQ-v6-27@gated-at.bofh.it>
In reply to#1216096
On 08/31/2015 01:38 AM, Peter Zijlstra wrote:
> On Sun, Aug 30, 2015 at 05:53:22PM +0200, Borislav Petkov wrote:
>> On Sat, Aug 29, 2015 at 11:19:14AM +0200, Ingo Molnar wrote:
>>> So let me withdraw my ack: the much more important question that I
>>> missed first time around, why is this reporting feature living in
>>> hwmon, not in perf? We have energy reporting facilities in perf that
>>> this should be synced to.
>>
>> Because there's already fam15h_power driver which is exactly for that.
>> Making it part of perf is then a question of cat-ting the same sysfs
>> file twice, at the beginning and at the end of the trace, which is
>> trivial.
>
> That don't make sense.
>
> Looking at the BKDG Fam 15h 60h-6Fh these MSRs are per compute unit.
> This means you can do much finer grained measurements than system wide
> -- which is all hwmon seems capable of.
>

Is it ? Why ?

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216183

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-31 15:40 +0200
Message-ID<q3xtv-Gh-1@gated-at.bofh.it>
In reply to#1216182
On Mon, Aug 31, 2015 at 06:26:00AM -0700, Guenter Roeck wrote:
> On 08/31/2015 01:38 AM, Peter Zijlstra wrote:
> >On Sun, Aug 30, 2015 at 05:53:22PM +0200, Borislav Petkov wrote:
> >>On Sat, Aug 29, 2015 at 11:19:14AM +0200, Ingo Molnar wrote:
> >>>So let me withdraw my ack: the much more important question that I
> >>>missed first time around, why is this reporting feature living in
> >>>hwmon, not in perf? We have energy reporting facilities in perf that
> >>>this should be synced to.
> >>
> >>Because there's already fam15h_power driver which is exactly for that.
> >>Making it part of perf is then a question of cat-ting the same sysfs
> >>file twice, at the beginning and at the end of the trace, which is
> >>trivial.
> >
> >That don't make sense.
> >
> >Looking at the BKDG Fam 15h 60h-6Fh these MSRs are per compute unit.
> >This means you can do much finer grained measurements than system wide
> >-- which is all hwmon seems capable of.
> >
> 
> Is it ? Why ?

Dunno, because there's that big old loop iterating all cpus?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216192

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-31 16:00 +0200
Message-ID<q3xMR-13i-3@gated-at.bofh.it>
In reply to#1216183
On 08/31/2015 06:38 AM, Peter Zijlstra wrote:
> On Mon, Aug 31, 2015 at 06:26:00AM -0700, Guenter Roeck wrote:
>> On 08/31/2015 01:38 AM, Peter Zijlstra wrote:
>>> On Sun, Aug 30, 2015 at 05:53:22PM +0200, Borislav Petkov wrote:
>>>> On Sat, Aug 29, 2015 at 11:19:14AM +0200, Ingo Molnar wrote:
>>>>> So let me withdraw my ack: the much more important question that I
>>>>> missed first time around, why is this reporting feature living in
>>>>> hwmon, not in perf? We have energy reporting facilities in perf that
>>>>> this should be synced to.
>>>>
>>>> Because there's already fam15h_power driver which is exactly for that.
>>>> Making it part of perf is then a question of cat-ting the same sysfs
>>>> file twice, at the beginning and at the end of the trace, which is
>>>> trivial.
>>>
>>> That don't make sense.
>>>
>>> Looking at the BKDG Fam 15h 60h-6Fh these MSRs are per compute unit.
>>> This means you can do much finer grained measurements than system wide
>>> -- which is all hwmon seems capable of.
>>>
>>
>> Is it ? Why ?
>
> Dunno, because there's that big old loop iterating all cpus?
>

What does that have to do with 'hwmon' ? The current implementation in the
driver may not be a good idea, and maybe for good reasons; I can not
comment on that. However, you concluded from that implementation that
hwmon, the subsystem, would not be able to support 'much finer grained
measurements than system wide'. I would like to understand how you reached
that conclusion.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216241

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-31 17:00 +0200
Message-ID<q3yIX-2rV-33@gated-at.bofh.it>
In reply to#1216192
On Mon, Aug 31, 2015 at 06:53:58AM -0700, Guenter Roeck wrote:

> What does that have to do with 'hwmon' ? The current implementation in the
> driver may not be a good idea, and maybe for good reasons; I can not
> comment on that. However, you concluded from that implementation that
> hwmon, the subsystem, would not be able to support 'much finer grained
> measurements than system wide'. I would like to understand how you reached
> that conclusion.

Purely going on what this driver does here. I assumed it was
representative of things, if not then good.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216249

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-31 17:20 +0200
Message-ID<q3z2i-33U-1@gated-at.bofh.it>
In reply to#1216241
On 08/31/2015 07:57 AM, Peter Zijlstra wrote:
> On Mon, Aug 31, 2015 at 06:53:58AM -0700, Guenter Roeck wrote:
>
>> What does that have to do with 'hwmon' ? The current implementation in the
>> driver may not be a good idea, and maybe for good reasons; I can not
>> comment on that. However, you concluded from that implementation that
>> hwmon, the subsystem, would not be able to support 'much finer grained
>> measurements than system wide'. I would like to understand how you reached
>> that conclusion.
>
> Purely going on what this driver does here. I assumed it was
> representative of things, if not then good.
>
Corollary: If a driver in subsystem X doesn't support interrupts,
it is a fair conclusion that subsystem X doesn't support interrupts.

The hwmon subsystem may have its limitations, and for sure would deserve
an overhaul, but at the same time people should refrain from bashing it
for no good reason.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216287

FromBorislav Petkov <bp@alien8.de>
Date2015-08-31 18:10 +0200
Message-ID<q3zOG-4hj-21@gated-at.bofh.it>
In reply to#1216096
On Mon, Aug 31, 2015 at 10:38:21AM +0200, Peter Zijlstra wrote:
> Looking at the BKDG Fam 15h 60h-6Fh these MSRs are per compute unit.
> This means you can do much finer grained measurements than system wide

Well, we can do finer-grained if needed. I'm all for everything which
has a good use case. The use case we had in mind here was the physical
processor power consumption for a time period.

> -- which is all hwmon seems capable of.

I guess we can do both - perf and hwmon. I don't see why not.

> Not to mention the proposed code is horrible, who in their right mind
> does two rdmsrl_safe_on_cpu() back to back.

That's a good point - I missed that during previous review. Rui, please
put the rdmsrl_safe_on_cpu() accesses in a separate function which you
run on a particular CPU, for your next version.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216302

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-31 18:20 +0200
Message-ID<q3zYn-4sz-31@gated-at.bofh.it>
In reply to#1216287
On 08/31/2015 09:06 AM, Borislav Petkov wrote:
> On Mon, Aug 31, 2015 at 10:38:21AM +0200, Peter Zijlstra wrote:
>> Looking at the BKDG Fam 15h 60h-6Fh these MSRs are per compute unit.
>> This means you can do much finer grained measurements than system wide
>
> Well, we can do finer-grained if needed. I'm all for everything which
> has a good use case. The use case we had in mind here was the physical
> processor power consumption for a time period.
>
>> -- which is all hwmon seems capable of.
>
> I guess we can do both - perf and hwmon. I don't see why not.
>
>> Not to mention the proposed code is horrible, who in their right mind
>> does two rdmsrl_safe_on_cpu() back to back.
>
> That's a good point - I missed that during previous review. Rui, please
> put the rdmsrl_safe_on_cpu() accesses in a separate function which you
> run on a particular CPU, for your next version.
>
... and maybe work with Peter to address the other hotplug related issues.

It might also be worthwhile thinking about per-CU attributes, if that
provides any value (Peter's comments suggested that this might be the case).

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216439

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-31 22:50 +0200
Message-ID<q3EbE-1Wt-19@gated-at.bofh.it>
In reply to#1216302
On Mon, Aug 31, 2015 at 09:19:20AM -0700, Guenter Roeck wrote:
> On 08/31/2015 09:06 AM, Borislav Petkov wrote:

> >That's a good point - I missed that during previous review. Rui, please
> >put the rdmsrl_safe_on_cpu() accesses in a separate function which you
> >run on a particular CPU, for your next version.
> >
> ... and maybe work with Peter to address the other hotplug related issues.
> 
> It might also be worthwhile thinking about per-CU attributes, if that
> provides any value (Peter's comments suggested that this might be the case).

Yeah, so it would allow measuring the power of a subset of compute
units. Typically only useful if you've partitioned your workload. But
since the hardware trivially supports it, its a waste to not expose it.

(Note that its not per-cpu, its per compute unit. What we do with perf
is export a cpumask)

My biggest problem is that all this is user readable and unthrottled. It
basically allows DoS (perf does not typically allow user access to CPU
wide resources).

Imagine joe user doing:

	for ((i=0; i<1000; i++)); do
		(while :; do cat /sys/foo/file > /dev/null ; done) &
	done

Even when contained to a subset of CPUs, that will cause an IPI storm on
all (/2) CPUs, even if you've tried really hard to keep users away from
some of them (see the above partitioning) because you're running some
important RT workload or whatnot.


As to hotplug, if you unplug any of the even numbered CPUs the whole
thing bails and returns 0, even if the corresponding odd CPU of the
compute unit it still online and perfectly capable of accessing the MSR.


As to relying on CPU numbering, maybe I should go write an APICID -> cpu
number randomizer, just for kicks to see what else fails.

We have topology information and cpumasks aplenty for things like this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216457

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-31 23:30 +0200
Message-ID<q3EOm-2Vg-19@gated-at.bofh.it>
In reply to#1216439
On 08/31/2015 01:44 PM, Peter Zijlstra wrote:
> On Mon, Aug 31, 2015 at 09:19:20AM -0700, Guenter Roeck wrote:
>> On 08/31/2015 09:06 AM, Borislav Petkov wrote:
>
>>> That's a good point - I missed that during previous review. Rui, please
>>> put the rdmsrl_safe_on_cpu() accesses in a separate function which you
>>> run on a particular CPU, for your next version.
>>>
>> ... and maybe work with Peter to address the other hotplug related issues.
>>
>> It might also be worthwhile thinking about per-CU attributes, if that
>> provides any value (Peter's comments suggested that this might be the case).
>
> Yeah, so it would allow measuring the power of a subset of compute
> units. Typically only useful if you've partitioned your workload. But
> since the hardware trivially supports it, its a waste to not expose it.
>
> (Note that its not per-cpu, its per compute unit. What we do with perf
> is export a cpumask)
>
> My biggest problem is that all this is user readable and unthrottled. It
> basically allows DoS (perf does not typically allow user access to CPU
> wide resources).
>
> Imagine joe user doing:
>
> 	for ((i=0; i<1000; i++)); do
> 		(while :; do cat /sys/foo/file > /dev/null ; done) &
> 	done
>
> Even when contained to a subset of CPUs, that will cause an IPI storm on
> all (/2) CPUs, even if you've tried really hard to keep users away from
> some of them (see the above partitioning) because you're running some
> important RT workload or whatnot.
>

That is a matter of driver implementation. Very commonly hwmon drivers
implement value caching, where data from the device is only read at
minimum intervals, and the cached values are reported if the information
is polled too rapidly. For the most part, that is used with i2c temperature
sensors, which tend to update their readings only a few times per second
anyway, but the same mechanism can (and possibly should) be used in
situations like this.

At some point I would like to move the caching mechanism into the hwmon core,
but that is going to take a while.

>
> As to hotplug, if you unplug any of the even numbered CPUs the whole
> thing bails and returns 0, even if the corresponding odd CPU of the
> compute unit it still online and perfectly capable of accessing the MSR.
>

Ah yes, I recall there was a similar problem in the coretemp driver
some time ago.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216940

FromBorislav Petkov <bp@alien8.de>
Date2015-09-01 18:00 +0200
Message-ID<q3W8y-2Bi-15@gated-at.bofh.it>
In reply to#1216457
On Mon, Aug 31, 2015 at 02:24:08PM -0700, Guenter Roeck wrote:
> That is a matter of driver implementation. Very commonly hwmon drivers
> implement value caching, where data from the device is only read at
> minimum intervals, and the cached values are reported if the information
> is polled too rapidly.

I don't see how we can use cached values in that case as they'd be
basically a lie as to how much power the CU/processor has consumed. I
guess it would be fairer to the user to warn instead and not issue any
values or to simply delay the read to a min timeout...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216943

FromGuenter Roeck <linux@roeck-us.net>
Date2015-09-01 18:10 +0200
Message-ID<q3Wie-31U-11@gated-at.bofh.it>
In reply to#1216940
On 09/01/2015 08:56 AM, Borislav Petkov wrote:
> On Mon, Aug 31, 2015 at 02:24:08PM -0700, Guenter Roeck wrote:
>> That is a matter of driver implementation. Very commonly hwmon drivers
>> implement value caching, where data from the device is only read at
>> minimum intervals, and the cached values are reported if the information
>> is polled too rapidly.
>
> I don't see how we can use cached values in that case as they'd be
> basically a lie as to how much power the CU/processor has consumed. I
> guess it would be fairer to the user to warn instead and not issue any
> values or to simply delay the read to a min timeout...
>

This is all a matter of ABI description. I personally don't see the
harm in caching values for up to <n> milliseconds and documenting that
readings may reflect power consumption from <n> milliseconds ago.
However, I don't mind any other solution either if that is considered
unacceptable, as long as that solution doesn't break the ABI.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web