Re: [PATCH v7 45/49] x86,fs/resctrl: Move the resctrl filesystem code to live in /fs/resctrl
From: James Morse
Date: Fri Mar 14 2025 - 13:42:50 EST
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