Messages in this thread | | | Date | Mon, 26 Oct 2015 05:36:45 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v2 06/10] hwmon: (fam15h_power) Add ptsc counter value for accumulated power |
| |
On Mon, Oct 26, 2015 at 11:37:23AM +0800, Huang Rui wrote: > On Fri, Oct 23, 2015 at 06:59:19AM -0700, Guenter Roeck wrote: > > On 10/19/2015 07:28 PM, Huang Rui wrote: > > >PTSC is the performance timestamp counter value in a cpu core and the > > >cores in one compute unit have the fixed frequency. So it picks up the > > >performance timestamp counter value of the first core per compute unit > > >to measure the interval for average power per compute unit. > > > > > >Signed-off-by: Huang Rui <ray.huang@amd.com> > > >Cc: Borislav Petkov <bp@alien8.de> > > >Cc: Guenter Roeck <linux@roeck-us.net> > > >Cc: Peter Zijlstra <peterz@infradead.org> > > >Cc: Ingo Molnar <mingo@kernel.org> > > >--- > > > arch/x86/include/asm/msr-index.h | 1 + > > > drivers/hwmon/fam15h_power.c | 5 +++++ > > > 2 files changed, 6 insertions(+)
...
> > >@@ -132,6 +134,9 @@ static void do_read_registers_on_cu(void *_data) > > > > > > WARN_ON(rdmsrl_safe(MSR_F15H_CU_PWR_ACCUMULATOR, > > > &data->cu_acc_power[cu])); > > >+ > > >+ WARN_ON(rdmsrl_safe(MSR_F15H_PTSC, > > >+ &data->cpu_sw_pwr_ptsc[cu])); > > > } > > > > I am not really happy with those WARN_ON, or even an error message. > > If the error is seen, it may be persistent. > > > > If an error check is really needed here, it might make more sense to store > > the read error and return it to user space if the respective sysfs attribute > > is read. > > > > I am OK with removing WARN_ON here. Boris, if you also agree with it,
The real question should be: are those MSR accesses behind a CPUID flag check which guarantees their existence?
If so, you don't really need WARN_ONs. And the MSR accesses better be behind a CPUID flag anyway because reading non-existent MSRs is pretty much pure waste of energy.
-- Regards/Gruss, Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
| |