[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a7122237-969d-4757-8c93-af261a8d69d1@arm.com>
Date: Thu, 4 Jul 2024 17:40:56 +0100
From: James Morse <james.morse@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, x86@...nel.org,
linux-kernel@...r.kernel.org
Cc: Fenghua Yu <fenghua.yu@...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>, Shaopeng Tan <tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH v3 36/38] fs/resctrl: Add boiler plate for external
resctrl code
Hi Reinette,
On 28/06/2024 17:54, Reinette Chatre wrote:
> On 6/14/24 8:00 AM, James Morse wrote:
>> Add Makefile and Kconfig for fs/resctrl. Add ARCH_HAS_CPU_RESCTRL
>> for the common parts of the resctrl interface and make X86_CPU_RESCTRL
>> depend on this.
>>
>> Adding an include of asm/resctrl.h to linux/resctrl.h allows some
>> of the files to switch over to using this header instead.
>> diff --git a/fs/resctrl/Kconfig b/fs/resctrl/Kconfig
>> new file mode 100644
>> index 000000000000..a5fbda54d32f
>> --- /dev/null
>> +++ b/fs/resctrl/Kconfig
>> @@ -0,0 +1,36 @@
>> +config RESCTRL_FS
>> + bool "CPU Resource Control Filesystem (resctrl)"
>> + depends on ARCH_HAS_CPU_RESCTRL
>> + select KERNFS
>> + select PROC_CPU_RESCTRL if PROC_FS
>> + help
>> + Some architectures provide hardware facilities to group tasks and
>> + monitor and control their usage of memory system resources such as
>> + caches and memory bandwidth. Examples of such facilities include
>> + Intel's Resource Director Technology (Intel(R) RDT) and AMD's
>> + Platform Quality of Service (AMD QoS).
>> +
>> + If your system has the necessary support and you want to be able to
>> + assign tasks to groups and manipulate the associated resource
>> + monitors and controls from userspace, say Y here to get a mountable
>> + 'resctrl' filesystem that lets you do just that.
>> +
>> + If nothing mounts or prods the 'resctrl' filesystem, resource
>> + controls and monitors are left in a quiescent, permissive state.
>> +
>> + If unsure, it is safe to say N.
>> +
>
> Will user ever get opportunity to say "Y" or "N"?
> It looks to me that
> RESCTRL_FS will be "forced" on user as it is selected by the arch specific
> config X86_CPU_RESCTRL and be invisble otherwise because of the dependency
> on ARCH_HAS_CPU_RESCTRL.
I did it like this so that this change is invisible for x86 config files on the principle
of 'least noise'. Users can't enable RDT but disable resctrl today.
It isn't actually possible to enable RDT and disable resctrl until after the code has been
split from the architecture code.
I have ended up supporting this for MPAM - you can enable the architecture's MPAM code and
the driver, but not resctrl. This will eventually be for in-kernel users of resources that
resctrl doesn't understand.
> The text about when to select "Y" or "N" thus does
> not look practical to me and it may be helpful to instead provide
> information about when it is selected? I do not know the customs for this
> text and if it is intended to document any future usages also.
I think Dave wrote this text because its traditional for Kconfig options to say this.
Describing when it is selected gets messy as this varies by architecture, and Kconfig can
already tell you this:
| Selected by [y]:
│ - X86_CPU_RESCTRL [=y] && X86 [=y] && (CPU_SUP_INTEL [=y] || CPU_SUP_AMD [=y]) &&
MISC_FILESYSTEMS [=y]
I don't think it makes sense for resctrl to be enabled/disabled independently on x86.
If you want to support this, we need a few more IS_ENABLED() checks and stubs to make it
build. The only reason I can see to do it is to ensure the architecture code is self
contained.
I'll reword this as "On architectures where this can be disabled independently, it is safe
to say N".
Thanks,
James
Powered by blists - more mailing lists