linux-kernel - Re: [PATCH v7 45/49] x86,fs/resctrl: Move the resctrl filesystem code to live in /fs/resctrl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <97aeaf87-b2db-4efb-9d81-43769c6f27b0@arm.com>
Date: Fri, 14 Mar 2025 17:42:15 +0000
From: James Morse <james.morse@....com>
To: Fenghua Yu <fenghuay@...dia.com>, x86@...nel.org,
 linux-kernel@...r.kernel.org
Cc: Reinette Chatre <reinette.chatre@...el.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, H Peter Anvin <hpa@...or.com>,
 Babu Moger <Babu.Moger@....com>, shameerali.kolothum.thodi@...wei.com,
 D Scott Phillips OS <scott@...amperecomputing.com>,
 carl@...amperecomputing.com, lcherian@...vell.com,
 bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
 baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
 Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
 dfustini@...libre.com, amitsinght@...vell.com,
 David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
 Dave Martin <dave.martin@....com>, Koba Ko <kobak@...dia.com>,
 Shanker Donthineni <sdonthineni@...dia.com>
Subject: Re: [PATCH v7 45/49] x86,fs/resctrl: Move the resctrl filesystem code
 to live in /fs/resctrl

Hi Fenghua,

On 06/03/2025 20:35, Fenghua Yu wrote:
> On 2/28/25 11:59, James Morse wrote:
>> Resctrl is a filesystem interface to hardware that provides cache
>> allocation policy and bandwidth control for groups of tasks or CPUs.
>>
>> To support more than one architecture, resctrl needs to live in /fs/.
>>
>> Move the code that is concerned with the filesystem interface to
>> /fs/resctrl.

>>   14 files changed, 7535 insertions(+), 7285 deletions(-)

(this patch is large - please trim it in your reply!)

[...]


>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor_trace.h b/arch/x86/kernel/cpu/resctrl/
>> monitor_trace.h
>> index ade67daf42c2..b5a142dd0f0e 100644
>> --- a/arch/x86/kernel/cpu/resctrl/monitor_trace.h
>> +++ b/arch/x86/kernel/cpu/resctrl/monitor_trace.h
>> @@ -7,2m5 +7,11 @@
>>     #include <linux/tracepoint.h>
>>   -TRACE_EVENT(mon_llc_occupancy_limbo,
>> -        TP_PROTO(u32 ctrl_hw_id, u32 mon_hw_id, int domain_id, u64 llc_occupancy_bytes),
>> -        TP_ARGS(ctrl_hw_id, mon_hw_id, domain_id, llc_occupancy_bytes),
>> -        TP_STRUCT__entry(__field(u32, ctrl_hw_id)
>> -                 __field(u32, mon_hw_id)
>> -                 __field(int, domain_id)
>> -                 __field(u64, llc_occupancy_bytes)),
>> -        TP_fast_assign(__entry->ctrl_hw_id = ctrl_hw_id;
>> -               __entry->mon_hw_id = mon_hw_id;
>> -               __entry->domain_id = domain_id;
>> -               __entry->llc_occupancy_bytes = llc_occupancy_bytes;),
>> -        TP_printk("ctrl_hw_id=%u mon_hw_id=%u domain_id=%d llc_occupancy_bytes=%llu",
>> -              __entry->ctrl_hw_id, __entry->mon_hw_id, __entry->domain_id,
>> -              __entry->llc_occupancy_bytes)
>> -       );
>> -
>>   #endif /* _FS_RESCTRL_MONITOR_TRACE_H */
>>     #undef TRACE_INCLUDE_PATH
>>   #define TRACE_INCLUDE_PATH .
>> +
>>   #define TRACE_INCLUDE_FILE monitor_trace
>> +
>>   #include <trace/define_trace.h>

> Similarly this file doesn't define any trace event. make W=1 complains it:
> 
>                 from arch/x86/kernel/cpu/resctrl/monitor.c:32:
> ./include/trace/stages/init.h:2:23: error: ‘str__resctrl__trace_system_name’ defined but
> not used [-Werror=unused-const-variable=]
>     2 | #define __app__(x, y) str__##x##y
>       |                       ^~~~~
> ./include/trace/stages/init.h:3:21: note: in expansion of macro ‘__app__’
>     3 | #define __app(x, y) __app__(x, y)
>       |                     ^~~~~~~
> ./include/trace/stages/init.h:5:29: note: in expansion of macro ‘__app’
>     5 | #define TRACE_SYSTEM_STRING __app(TRACE_SYSTEM_VAR,__trace_system_name)
>       |                             ^~~~~
> ./include/trace/stages/init.h:8:27: note: in expansion of macro ‘TRACE_SYSTEM_STRING’
>     8 |         static const char TRACE_SYSTEM_STRING[] =       \
>       |                           ^~~~~~~~~~~~~~~~~~~
> ./include/trace/stages/init.h:11:1: note: in expansion of macro ‘TRACE_MAKE_SYSTEM_STR’
>    11 | TRACE_MAKE_SYSTEM_STR();
>       | ^~~~~~~~~~~~~~~~~~~~~
>   CC      fs/proc/consoles.o
> cc1: all warnings being treated as errors
> make[6]: *** [scripts/Makefile.build:207: arch/x86/kernel/cpu/resctrl/monitor.o] Error 1
> make[6]: *** Waiting for unfinished jobs....


> The above two compilation errors only happen when first 45 patches are applied. The whole
> patch set won't have the errors because patch 46 removes these empty trace events.
> 
> The fix is simple: just merge this patch and patch 46 together. Then there is no empty
> trace event in trace.h files and thus no compilation errors.

The intent is that these last few patches are merged together by the person that applies
them. I thought I'd described that in the cover-letter, but that text got lost.

It's like this because this patch is too large to review - but it is generated by a
script. If I merge these patches together, then its even harder to have confidence that
some hunk isn't getting reverted to an older version.


Thanks,

James