On 2/9/24 12:44, Tony Luck wrote:
> On Fri, Feb 09, 2024 at 09:28:16AM -0600, Moger, Babu wrote:
>>> enum resctrl_res_level {
>>> + RDT_RESOURCE_L3_MON,
>>> RDT_RESOURCE_L3,
>>
>> How about?
>> RDT_RESOURCE_L3,
>> RDT_RESOURCE_L3_MON,
>
> Does the order matter? I put the L3_MON one first because historically
> cache occupancy was the first resctrl tool. But if you have a better
> argument for the order, then I can change it.
That is fine. No need to change.
>
>>> RDT_RESOURCE_L2,
>>> RDT_RESOURCE_MBA,
>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>>> index aa9810a64258..c50f55d7790e 100644
>>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>>> @@ -60,6 +60,16 @@ mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m,
>>> #define domain_init(id) LIST_HEAD_INIT(rdt_resources_all[id].r_resctrl.domains)
>>>
>>> struct rdt_hw_resource rdt_resources_all[] = {
>>> + [RDT_RESOURCE_L3_MON] =
>>> + {
>>> + .r_resctrl = {
>>> + .rid = RDT_RESOURCE_L3_MON,
>>> + .name = "L3",
>>
>> L3_MON ?
>
> That was my first choice too. But I found:
>
> $ ls /sys/fs/resctrl/info
> L3 L3_MON_MON last_cmd_status MB
>
> This would be easy to fix ... just change this code to not append
> an extra "_MON" to the directory name:
>
> for_each_mon_capable_rdt_resource(r) {
> fflags = r->fflags | RFTYPE_MON_INFO;
> sprintf(name, "%s_MON", r->name);
> ret = rdtgroup_mkdir_info_resdir(r, name, fflags);
> if (ret)
> goto out_destroy;
> }
>
> But I also saw this:
> $ ls /sys/fs/resctrl/mon_data/
> mon_L3_MON_00 mon_L3_MON_01
>
> which didn't seem to have an easy fix. So I took the easy route and left
> the ".name" field as "L3_MON".
>
Ok. Sounds good.
--
Thanks
Babu Moger
