Re: [PATCH v7 10/13] selftests/resctrl: Add vendor detection mechanism

[James Morse]

Hi AndrÃ,

On 14/05/2019 20:40, Andrà Przywara wrote:
> On 14/05/2019 18:20, James Morse wrote:
>> On 10/05/2019 18:39, Andre Przywara wrote:
>>> On Sat,  9 Feb 2019 18:50:39 -0800
>>> Fenghua Yu <fenghua.yu@xxxxxxxxx> wrote:
>>>> From: Babu Moger <babu.moger@xxxxxxx>
>>>>
>>>> RESCTRL feature is supported both on Intel and AMD now. Some features
>>>> are implemented differently. Add vendor detection mechanism. Use the vendor
>>>> check where there are differences.
>>>
>>> I don't think vendor detection is the right approach. The Linux userland
>>> interface should be even architecture agnostic, not to speak of different
>>> vendors.
>>>
>>> But even if we need this for some reason ...

>> What do we need it for? Surely it indicates something is wrong with the kernel interface
>> if you need to know which flavour of CPU this is.
> 
> As you mentioned, we should not need it. I just couldn't find a better
> way (yet) to differentiate between L3 cache ID and physical package ID
> (see patch 11/13). So this is a kludge for now to not break this
> particular code.

[0]? That's broken. It needs to take the 'cache/index?/id' field, and not hard-code '3',
search each 'cache/index?/level' instead.


Documentation/x86/resctrl_ui.rst's "Cache IDs" section says:
| On current generation systems there is one L3 cache per socket and L2
| caches are generally just shared by the hyperthreads on a core, but this
| isn't an architectural requirement.
[...]
| So instead of using "socket" or "core" to define the set of logical cpus
| sharing a resource we use a "Cache ID"
[...]
| To find the ID for each logical CPU look in
| /sys/devices/system/cpu/cpu*/cache/index*/id

arch/x86/kernel/cpu/restrl/core.c:domain_add_cpu() pulls the domain-id out of struct
cacheinfo:
|	int id = get_cache_id(cpu, r->cache_level);

drivers/base/cacheinfo.c has some macro-foliage that exports this same field via sysfs,
and arch/x86/kernel/cpu/restrl/ctrlmondata.c:parse_line() matches that id against the
value user-space provides in the schemata.

(we've got some horrible code for arm64 to make this work without 'cache id' as a hardware
property!)

On x86 these numbers are of the order 0,1,2, so its very likely physical_package_id and
cache_id alias, and you get away with it.


> Out of curiosity: Is there any userland tool meant to control the
> resources? I guess poking around in sysfs is not how admins are expected
> to use this?

I've come across:
https://github.com/intel/intel-cmt-cat/

but I've never even cloned it. The rdtset man page has:
| With --iface-os (-I) parameter, rdtset uses resctrl filesystem (/sys/fs/resctrl)
| instead of accessing MSRs directly.


> This tool would surely run into the same problems, which somewhat tell
> me that the interface is not really right.

At the moment its not as-documented or as the kernel is using those numbers.

I assume this is something that changed in resctrl when it was merged, and this selftest
tool just needs updating.


Thanks,

James

[0] https://lkml.org/lkml/2019/2/9/384