2017-06-08 18:24:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] Implement sysfs based cpuinfo for x86 cpus.

On Thu, Jun 08, 2017 at 06:56:14PM +0200, Felix Schnizlein wrote:
> A few entries have been omitted, due to already existing information in
> sysfs:
>
> - microcode -> /sys/devices/system/cpu/cpu*/microcode/version
> - cache -> /sys/devices/system/cpu/cpu*/cache/index*/size
> - core_id -> /sys/devices/system/cpu/cpu*/topology/core_id
> - siblings -> /sys/devices/system/cpu/cpu*/topology/core_siblings_list
> - pyshical_id -> /sys/devices/system/cpu/cpu*/topology/physical_package_id
>
> Following attributes have been removed due to obsolete information or
> legacy processors: wp, power management, f00f bug, coma bug, fdiv bug
>
> Signed-off-by: Thomas Renninger <[email protected]>

Again, no signoff from you?

> ---
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/cpuinfo_sysfs.c | 166 ++++++++++++++++++++++++++++++++++++++++
> drivers/base/cpuinfo.c | 4 -
> 3 files changed, 167 insertions(+), 4 deletions(-)
> create mode 100644 arch/x86/kernel/cpuinfo_sysfs.c
>

When you add new sysfs entries, you have to also add new
Documentation/ABI/ entries.

thanks,

greg k-h


2017-06-09 13:28:53

by Thomas Renninger

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] Implement sysfs based cpuinfo for x86 cpus.

On Thursday, June 08, 2017 08:24:01 PM Greg KH wrote:
> On Thu, Jun 08, 2017 at 06:56:14PM +0200, Felix Schnizlein wrote:
> > ---
> > arch/x86/kernel/Makefile | 1 +
> > arch/x86/kernel/cpuinfo_sysfs.c | 166
++++++++++++++++++++++++++++++++++++++++
> > drivers/base/cpuinfo.c | 4 -
> > 3 files changed, 167 insertions(+), 4 deletions(-)
> > create mode 100644 arch/x86/kernel/cpuinfo_sysfs.c
> >
>
> When you add new sysfs entries, you have to also add new
> Documentation/ABI/ entries.

This one seem to be rather unmaintained?
There even is:
Documentation/ABI/stable/sysfs-devices-system-cpu
this patchset would have to add:
Documentation/ABI/stable/sysfs-devices-system-cpu-cpuinfo
then.

But:
Documentation/ABI/stable/sysfs-devices-system-cpu
describes
/sys/devices/system/cpu/dscr_default
and
/sys/devices/system/cpu/cpu[0-9]+/dscr

which I have never seen.

Much more important values in there like:
/sys/devices/system/cpu/topology
/sys/devices/system/cpu/microcode
/sys/devices/system/cpu/cpuidle
/sys/devices/system/cpu/cpufreq
/sys/devices/system/cpu/{online,offline}

are not described there at all.

I wonder whether .../cpu/dscr_default still exists in the
kernel, a quick grep did not reveal anything.
A
Source:
tag would be a nice non-optional addition in the README.

We can also send definitions for topology/microcode/cache/..
to catch up a bit there again, not sure that makes much sense.

Be aware that the output of /proc/cpuinfo is rather arbitrary
depending on architecure and some build options.

Thomas

2017-06-09 15:22:23

by Brice Goglin

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] Implement sysfs based cpuinfo for x86 cpus.



Le 09/06/2017 15:28, Thomas Renninger a ?crit :
> On Thursday, June 08, 2017 08:24:01 PM Greg KH wrote:
>> On Thu, Jun 08, 2017 at 06:56:14PM +0200, Felix Schnizlein wrote:
>>> ---
>>> arch/x86/kernel/Makefile | 1 +
>>> arch/x86/kernel/cpuinfo_sysfs.c | 166
> ++++++++++++++++++++++++++++++++++++++++
>>> drivers/base/cpuinfo.c | 4 -
>>> 3 files changed, 167 insertions(+), 4 deletions(-)
>>> create mode 100644 arch/x86/kernel/cpuinfo_sysfs.c
>>>
>> When you add new sysfs entries, you have to also add new
>> Documentation/ABI/ entries.
> This one seem to be rather unmaintained?
> There even is:
> Documentation/ABI/stable/sysfs-devices-system-cpu
> this patchset would have to add:
> Documentation/ABI/stable/sysfs-devices-system-cpu-cpuinfo
> then.
>
> But:
> Documentation/ABI/stable/sysfs-devices-system-cpu
> describes
> /sys/devices/system/cpu/dscr_default
> and
> /sys/devices/system/cpu/cpu[0-9]+/dscr
>
> which I have never seen.
>
> Much more important values in there like:
> /sys/devices/system/cpu/topology
> /sys/devices/system/cpu/microcode
> /sys/devices/system/cpu/cpuidle
> /sys/devices/system/cpu/cpufreq
> /sys/devices/system/cpu/{online,offline}

They are documented in
Documentation/ABI/testing/sysfs-devices-system-cpu (which doesn't
contain dscr)

Brice


>
> are not described there at all.
>
> I wonder whether .../cpu/dscr_default still exists in the
> kernel, a quick grep did not reveal anything.
> A
> Source:
> tag would be a nice non-optional addition in the README.
>
> We can also send definitions for topology/microcode/cache/..
> to catch up a bit there again, not sure that makes much sense.
>
> Be aware that the output of /proc/cpuinfo is rather arbitrary
> depending on architecure and some build options.
>
> Thomas