I worked out 2 patches to export cpu topology and cache info by sysfs.
The first patch is to export cpu topology info including below items
(attributes) which are similar to /proc/cpuinfo.
/sys/devices/system/cpu/cpuX/topology/physical_package_id(representing
the physical package id of cpu X)
/sys/devices/system/cpu/cpuX/topology/core_id (representing the cpu core
id to cpu X)
/sys/devices/system/cpu/cpuX/topology/thread_id (representing the cpu
thread id to cpu X)
/sys/devices/system/cpu/cpuX/topology/thread_siblings (representing the
thread siblings to cpu X)
/sys/devices/system/cpu/cpuX/topology/core_siblings (represeting the
core siblings to cpu X)
Signed-off-by: Zhang Yanmin <[email protected]>
Hello-
Zhang, Yanmin wrote:
> I worked out 2 patches to export cpu topology and cache info by sysfs.
>
> The first patch is to export cpu topology info including below items
> (attributes) which are similar to /proc/cpuinfo.
>
> /sys/devices/system/cpu/cpuX/topology/physical_package_id(representing
> the physical package id of cpu X)
> /sys/devices/system/cpu/cpuX/topology/core_id (representing the cpu core
> id to cpu X)
> /sys/devices/system/cpu/cpuX/topology/thread_id (representing the cpu
> thread id to cpu X)
> /sys/devices/system/cpu/cpuX/topology/thread_siblings (representing the
> thread siblings to cpu X)
> /sys/devices/system/cpu/cpuX/topology/core_siblings (represeting the
> core siblings to cpu X)
I haven't looked at the patches in detail, but I have a concern about
this approach. How is it that making new architecture-specific
attributes under cpu directories in sysfs is preferable to the already
architecture-specific format of /proc/cpuinfo and other proc entries?
If we're going to create a new user interface for exposing system
topology (cores and threads etc), I would like for it to be as
architecture-neutral as possible. We already do this for numa, for
example.
Nathan
Some comments on your patch ...
1) It's easier for others to read patches if they are inline text,
or at least attached as text, not as base64. See further the
kernel source file: Documentation/SubmittingPatches. If your
company's email client has difficulty attaching patches without
mangling them, then perhaps you will have better luck with a
dedicated patch sending program, such as one I support:
http://www.speakeasy.org/~pj99/sgi/sendpatchset
2) > cpumask_scnprintf(buf, NR_CPUS+1, cpu_core_map[cpu]);
The 2nd arg, "NR_CPUS+1", is wrong. It should be the length
of the buffer (1st arg, "buf"). Unfortunately, you probably
aren't passed that length by sysfs. Your routine was likely
passed a page, so assuming a size of PAGE_SIZE/2 would work
(big enough to print a cpumask, small enough not to allow
buffer overrun.)
3) The patch needs to include reasonable documentation (not
just the patch header that goes in the source control log,
but also documentation that will into the source file and/or
into the Documentation directory.) Unfortunately, it seems
that the rest of /sys/devices/system is not directly documented
under Documentation, except as it pertains to such subjects as
cpufreq, laptop, numastat and hotplug. So until someone takes
on the challenge of documenting the rest of this /sys hierarchy,
I see no obvious place to add such items as you propose.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
>>-----Original Message-----
>>From: Paul Jackson [mailto:[email protected]]
>>Sent: 2005??12??14?? 18:12
>>To: Zhang, Yanmin
>>Cc: [email protected]; Pallipadi, Venkatesh
>>Subject: Re: [PATCH 1/2] Export cpu info by sysfs
>>
>>Some comments on your patch ...
I really appreciate your comments.
>>
>>1) It's easier for others to read patches if they are inline text,
>> or at least attached as text, not as base64. See further the
>> kernel source file: Documentation/SubmittingPatches. If your
>> company's email client has difficulty attaching patches without
>> mangling them, then perhaps you will have better luck with a
>> dedicated patch sending program, such as one I support:
>> http://www.speakeasy.org/~pj99/sgi/sendpatchset
Thanks. I use outlook and tried many approaches before, but patches always have unexpected line breaks when I attach them as inline text. Anyway, I found a new approach to avoid that. Next time, I will paste patches as inline.
>>
>>2) > cpumask_scnprintf(buf, NR_CPUS+1, cpu_core_map[cpu]);
>>
>> The 2nd arg, "NR_CPUS+1", is wrong. It should be the length
>> of the buffer (1st arg, "buf"). Unfortunately, you probably
>> aren't passed that length by sysfs. Your routine was likely
>> passed a page, so assuming a size of PAGE_SIZE/2 would work
>> (big enough to print a cpumask, small enough not to allow
>> buffer overrun.)
In theory, it's a problem which doesn't exist in fact. The smallest page size on IA64 is 4KB (default is 16KB) and cpumask_scnprintf uses hex format, so one page could store cpumask of (4K-1)*4 cpu. I can't imagine a machine has more than (4K-1)*4 cpu.
>>
>>3) The patch needs to include reasonable documentation (not
>> just the patch header that goes in the source control log,
>> but also documentation that will into the source file and/or
>> into the Documentation directory.) Unfortunately, it seems
>> that the rest of /sys/devices/system is not directly documented
>> under Documentation, except as it pertains to such subjects as
>> cpufreq, laptop, numastat and hotplug. So until someone takes
>> on the challenge of documenting the rest of this /sys hierarchy,
>> I see no obvious place to add such items as you propose.
I agree with you.
Yanmin wrote:
> cpumask_scnprintf(buf, NR_CPUS+1, cpu_core_map[cpu]);
Paul wrote:
> The 2nd arg, "NR_CPUS+1", is wrong. It should be the length
> of the buffer
Yanmin replied:
> In theory, it's a problem which doesn't exist in fact.
Aha - you are right. My confusion was that I had forgotten
the format used by cpumask_scnprintf, and thought it was one
of the other formats, not a hex mask, that could overflow
NR_CPUS+1 characters of output. Please nevermind my confusion.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401