[PATCH v6 41/42] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl

James Morse posted 42 patches 1 year ago
There is a newer version of this series
[PATCH v6 41/42] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by James Morse 1 year ago
Once the filesystem parts of resctrl move to fs/resctrl, it cannot rely
on definitions in x86's internal.h.

Move definitions in internal.h that need to be shared between the
filesystem and architecture code to header files that fs/resctrl can
include.

Doing this separately means the filesystem code only moves between files
of the same name, instead of having these changes mixed in too.

Co-developed-by: Dave Martin <Dave.Martin@arm.com>
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Signed-off-by: James Morse <james.morse@arm.com>
Tested-by: Carl Worth <carl@os.amperecomputing.com> # arm64
Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
Reviewed-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
Changes since v3:
 * Changed the number of hyphens at the end of the commit message.

Changes since v2:
 * Dropped the rfflags and some other defines from being moved.

Changes since v1:
 * Revert apparently unintentional duplication of a couple of variable
   declarations in <linux/resctrl.h>.

   No functional change.
---
 arch/x86/include/asm/resctrl.h         | 3 +++
 arch/x86/kernel/cpu/resctrl/core.c     | 5 +++++
 arch/x86/kernel/cpu/resctrl/internal.h | 9 ---------
 include/linux/resctrl_types.h          | 3 +++
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index 7a39728b0743..6eb7d5c94c7a 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -210,6 +210,9 @@ int resctrl_arch_measure_l2_residency(void *_plr);
 int resctrl_arch_measure_l3_residency(void *_plr);
 void resctrl_cpu_detect(struct cpuinfo_x86 *c);
 
+bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l);
+int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
+
 #else
 
 static inline void resctrl_arch_sched_in(struct task_struct *tsk) {}
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 6303c0ee0ae2..f2cd7ba39fcc 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -288,6 +288,11 @@ static void rdt_get_cdp_l2_config(void)
 	rdt_get_cdp_config(RDT_RESOURCE_L2);
 }
 
+bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l)
+{
+	return rdt_resources_all[l].cdp_enabled;
+}
+
 static void mba_wrmsr_amd(struct msr_param *m)
 {
 	struct rdt_hw_ctrl_domain *hw_dom = resctrl_to_arch_ctrl_dom(m->dom);
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index ee50b7375717..a569e31d75f6 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -16,8 +16,6 @@
 #define CQM_LIMBOCHECK_INTERVAL	1000
 
 #define MBM_CNTR_WIDTH_BASE		24
-#define MBM_OVERFLOW_INTERVAL		1000
-#define MAX_MBA_BW			100u
 #define MBA_IS_LINEAR			0x4
 #define MBM_CNTR_WIDTH_OFFSET_AMD	20
 
@@ -403,13 +401,6 @@ extern struct rdtgroup rdtgroup_default;
 extern struct dentry *debugfs_resctrl;
 extern enum resctrl_event_id mba_mbps_default_event;
 
-static inline bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l)
-{
-	return rdt_resources_all[l].cdp_enabled;
-}
-
-int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
-
 void arch_mon_domain_online(struct rdt_resource *r, struct rdt_mon_domain *d);
 
 /* CPUID.(EAX=10H, ECX=ResID=1).EAX */
diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
index 70226f5ab3e3..b84a6e0834a7 100644
--- a/include/linux/resctrl_types.h
+++ b/include/linux/resctrl_types.h
@@ -7,6 +7,9 @@
 #ifndef __LINUX_RESCTRL_TYPES_H
 #define __LINUX_RESCTRL_TYPES_H
 
+#define MAX_MBA_BW			100u
+#define MBM_OVERFLOW_INTERVAL		1000
+
 /* Reads to Local DRAM Memory */
 #define READS_TO_LOCAL_MEM		BIT(0)
 
-- 
2.39.2
Re: [PATCH v6 41/42] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by Reinette Chatre 11 months, 3 weeks ago
Hi James,

On 2/7/25 10:18 AM, James Morse wrote:
> Once the filesystem parts of resctrl move to fs/resctrl, it cannot rely
> on definitions in x86's internal.h.
> 
> Move definitions in internal.h that need to be shared between the
> filesystem and architecture code to header files that fs/resctrl can
> include.
> 
> Doing this separately means the filesystem code only moves between files
> of the same name, instead of having these changes mixed in too.
> 
> Co-developed-by: Dave Martin <Dave.Martin@arm.com>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> Tested-by: Carl Worth <carl@os.amperecomputing.com> # arm64
> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---

..

> ---
>  arch/x86/include/asm/resctrl.h         | 3 +++
>  arch/x86/kernel/cpu/resctrl/core.c     | 5 +++++
>  arch/x86/kernel/cpu/resctrl/internal.h | 9 ---------
>  include/linux/resctrl_types.h          | 3 +++
>  4 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index 7a39728b0743..6eb7d5c94c7a 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h
> @@ -210,6 +210,9 @@ int resctrl_arch_measure_l2_residency(void *_plr);
>  int resctrl_arch_measure_l3_residency(void *_plr);
>  void resctrl_cpu_detect(struct cpuinfo_x86 *c);
>  
> +bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l);
> +int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
> +
>  #else
>  
>  static inline void resctrl_arch_sched_in(struct task_struct *tsk) {}
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 6303c0ee0ae2..f2cd7ba39fcc 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -288,6 +288,11 @@ static void rdt_get_cdp_l2_config(void)
>  	rdt_get_cdp_config(RDT_RESOURCE_L2);
>  }
>  
> +bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l)
> +{
> +	return rdt_resources_all[l].cdp_enabled;
> +}
> +

This moves resctrl_arch_get_cdp_enabled() to arch/x86/kernel/cpu/resctrl/core.c
while resctrl_arch_set_cdp_enabled() is already in arch/x86/kernel/cpu/resctrl/rdtgroup.c.
Most of resctrl_arch_get_cdp_enabled()'s callers are 
in arch/x86/kernel/cpu/resctrl/rdtgroup.c so it seems appropriate to keep it with
its partner resctrl_arch_set_cdp_enabled()?

Reinette
Re: [PATCH v6 41/42] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by James Morse 11 months, 2 weeks ago
Hi Reinette,

On 20/02/2025 06:00, Reinette Chatre wrote:
> On 2/7/25 10:18 AM, James Morse wrote:
>> Once the filesystem parts of resctrl move to fs/resctrl, it cannot rely
>> on definitions in x86's internal.h.
>>
>> Move definitions in internal.h that need to be shared between the
>> filesystem and architecture code to header files that fs/resctrl can
>> include.
>>
>> Doing this separately means the filesystem code only moves between files
>> of the same name, instead of having these changes mixed in too.

>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>> index 6303c0ee0ae2..f2cd7ba39fcc 100644
>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>> @@ -288,6 +288,11 @@ static void rdt_get_cdp_l2_config(void)
>>  	rdt_get_cdp_config(RDT_RESOURCE_L2);
>>  }
>>  
>> +bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l)
>> +{
>> +	return rdt_resources_all[l].cdp_enabled;
>> +}
>> +
> 
> This moves resctrl_arch_get_cdp_enabled() to arch/x86/kernel/cpu/resctrl/core.c
> while resctrl_arch_set_cdp_enabled() is already in arch/x86/kernel/cpu/resctrl/rdtgroup.c.
> Most of resctrl_arch_get_cdp_enabled()'s callers are 
> in arch/x86/kernel/cpu/resctrl/rdtgroup.c so it seems appropriate to keep it with
> its partner resctrl_arch_set_cdp_enabled()?

Yup - that would make more sense.
(This will date back to when I was moving the code around by hand every release!)


Thanks,

James