[<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
Powered by blists - more mailing lists