1. Patchew
  2. linux
  3. [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
  4. View patch

[PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl

James Morse posted 31 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by James Morse 1 year, 10 months 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.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/x86/include/asm/resctrl.h         |  3 +++
 arch/x86/kernel/cpu/resctrl/core.c     |  5 ++++
 arch/x86/kernel/cpu/resctrl/internal.h | 36 --------------------------
 include/linux/resctrl.h                |  3 +++
 include/linux/resctrl_types.h          | 30 +++++++++++++++++++++
 5 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
index 491342f56811..746431c66fc4 100644
--- a/arch/x86/include/asm/resctrl.h
+++ b/arch/x86/include/asm/resctrl.h
@@ -218,6 +218,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 f94ad04023c3..c0fb2e22e110 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -306,6 +306,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 rdt_domain *d, struct msr_param *m, struct rdt_resource *r)
 {
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 56218193a8ba..0f7e3f10941b 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -15,12 +15,6 @@
 
 #define L2_QOS_CDP_ENABLE		0x01ULL
 
-#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
 
 #define RMID_VAL_ERROR			BIT_ULL(63)
@@ -210,29 +204,6 @@ struct rdtgroup {
 	struct pseudo_lock_region	*plr;
 };
 
-/* rdtgroup.flags */
-#define	RDT_DELETED		1
-
-/* rftype.flags */
-#define RFTYPE_FLAGS_CPUS_LIST	1
-
-/*
- * Define the file type flags for base and info directories.
- */
-#define RFTYPE_INFO			BIT(0)
-#define RFTYPE_BASE			BIT(1)
-#define RFTYPE_CTRL			BIT(4)
-#define RFTYPE_MON			BIT(5)
-#define RFTYPE_TOP			BIT(6)
-#define RFTYPE_RES_CACHE		BIT(8)
-#define RFTYPE_RES_MB			BIT(9)
-#define RFTYPE_DEBUG			BIT(10)
-#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
-#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
-#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
-#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
-#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
-
 /* List of all resource groups */
 extern struct list_head rdt_all_groups;
 
@@ -370,13 +341,6 @@ static inline struct rdt_resource *resctrl_inc(struct rdt_resource *res)
 	return &hw_res->r_resctrl;
 }
 
-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);
-
 /*
  * To return the common struct rdt_resource, which is contained in struct
  * rdt_hw_resource, walk the resctrl member of struct rdt_hw_resource.
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index f786ffceeda3..00cc0457af50 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -41,6 +41,9 @@ int proc_resctrl_show(struct seq_file *m,
  */
 #define RESCTRL_MAX_CBM			32
 
+extern unsigned int resctrl_rmid_realloc_limit;
+extern unsigned int resctrl_rmid_realloc_threshold;
+
 /**
  * struct pseudo_lock_region - pseudo-lock region information
  * @s:			Resctrl schema for the resource to which this
diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
index 4788bd95dac6..fe0b10b589c0 100644
--- a/include/linux/resctrl_types.h
+++ b/include/linux/resctrl_types.h
@@ -7,6 +7,36 @@
 #ifndef __LINUX_RESCTRL_TYPES_H
 #define __LINUX_RESCTRL_TYPES_H
 
+#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
+
+/* rdtgroup.flags */
+#define	RDT_DELETED		1
+
+/* rftype.flags */
+#define RFTYPE_FLAGS_CPUS_LIST	1
+
+/*
+ * Define the file type flags for base and info directories.
+ */
+#define RFTYPE_INFO			BIT(0)
+#define RFTYPE_BASE			BIT(1)
+#define RFTYPE_CTRL			BIT(4)
+#define RFTYPE_MON			BIT(5)
+#define RFTYPE_TOP			BIT(6)
+#define RFTYPE_RES_CACHE		BIT(8)
+#define RFTYPE_RES_MB			BIT(9)
+#define RFTYPE_DEBUG			BIT(10)
+#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
+#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
+#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
+#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
+#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
+
 /* Reads to Local DRAM Memory */
 #define READS_TO_LOCAL_MEM		BIT(0)
 
-- 
2.39.2
Re: [PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by Reinette Chatre 1 year, 10 months ago 
Hi James,

On 3/21/2024 9:51 AM, James Morse wrote:
...
> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> index 4788bd95dac6..fe0b10b589c0 100644
> --- a/include/linux/resctrl_types.h
> +++ b/include/linux/resctrl_types.h
> @@ -7,6 +7,36 @@
>  #ifndef __LINUX_RESCTRL_TYPES_H
>  #define __LINUX_RESCTRL_TYPES_H
>  
> +#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
> +
> +/* rdtgroup.flags */
> +#define	RDT_DELETED		1
> +
> +/* rftype.flags */
> +#define RFTYPE_FLAGS_CPUS_LIST	1
> +
> +/*
> + * Define the file type flags for base and info directories.
> + */
> +#define RFTYPE_INFO			BIT(0)
> +#define RFTYPE_BASE			BIT(1)
> +#define RFTYPE_CTRL			BIT(4)
> +#define RFTYPE_MON			BIT(5)
> +#define RFTYPE_TOP			BIT(6)
> +#define RFTYPE_RES_CACHE		BIT(8)
> +#define RFTYPE_RES_MB			BIT(9)
> +#define RFTYPE_DEBUG			BIT(10)
> +#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
> +#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
> +#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
> +#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
> +#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
> +
>  /* Reads to Local DRAM Memory */
>  #define READS_TO_LOCAL_MEM		BIT(0)
>  

Not all these new seem to belong in this file. Could you please confirm?

For example:
Earlier in series it was mentioned that struct rdtgroup is private to the
fs so having RDT_DELETED is unexpected as it implies access to struct rdtgroup.

CQM_LIMBOCHECK_INTERVAL seems private to the fs code, so too
RFTYPE_FLAGS_CPUS_LIST.

Reinette
Re: [PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by Dave Martin 1 year, 10 months ago 
On Mon, Apr 08, 2024 at 08:42:00PM -0700, Reinette Chatre wrote:
> Hi James,
> 
> On 3/21/2024 9:51 AM, James Morse wrote:
> ..
> > diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> > index 4788bd95dac6..fe0b10b589c0 100644
> > --- a/include/linux/resctrl_types.h
> > +++ b/include/linux/resctrl_types.h
> > @@ -7,6 +7,36 @@
> >  #ifndef __LINUX_RESCTRL_TYPES_H
> >  #define __LINUX_RESCTRL_TYPES_H
> >  
> > +#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
> > +
> > +/* rdtgroup.flags */
> > +#define	RDT_DELETED		1
> > +
> > +/* rftype.flags */
> > +#define RFTYPE_FLAGS_CPUS_LIST	1
> > +
> > +/*
> > + * Define the file type flags for base and info directories.
> > + */
> > +#define RFTYPE_INFO			BIT(0)
> > +#define RFTYPE_BASE			BIT(1)
> > +#define RFTYPE_CTRL			BIT(4)
> > +#define RFTYPE_MON			BIT(5)
> > +#define RFTYPE_TOP			BIT(6)
> > +#define RFTYPE_RES_CACHE		BIT(8)
> > +#define RFTYPE_RES_MB			BIT(9)
> > +#define RFTYPE_DEBUG			BIT(10)
> > +#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
> > +#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
> > +#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
> > +#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
> > +#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
> > +
> >  /* Reads to Local DRAM Memory */
> >  #define READS_TO_LOCAL_MEM		BIT(0)
> >  
> 
> Not all these new seem to belong in this file. Could you please confirm?
> 
> For example:
> Earlier in series it was mentioned that struct rdtgroup is private to the
> fs so having RDT_DELETED is unexpected as it implies access to struct rdtgroup.
> 
> CQM_LIMBOCHECK_INTERVAL seems private to the fs code, so too
> RFTYPE_FLAGS_CPUS_LIST.
> 
> Reinette
> 

I'll flag this for James to review.

These have to be moved out of the x86 private headers, but you're right
that some of them seem logically private to the resctrl core.

I guess some of these could move to fs/resctrl/internal.h?

OTOH, might it be preferable to keep all the flag definitions for a
given member together for ease of maintenance, even if some are for
resctrl internal use only?

Cheers
---Dave
Re: [PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by Reinette Chatre 1 year, 10 months ago 
Hi Dave,

On 4/11/2024 7:28 AM, Dave Martin wrote:
> On Mon, Apr 08, 2024 at 08:42:00PM -0700, Reinette Chatre wrote:
>> Hi James,
>>
>> On 3/21/2024 9:51 AM, James Morse wrote:
>> ..
>>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
>>> index 4788bd95dac6..fe0b10b589c0 100644
>>> --- a/include/linux/resctrl_types.h
>>> +++ b/include/linux/resctrl_types.h
>>> @@ -7,6 +7,36 @@
>>>  #ifndef __LINUX_RESCTRL_TYPES_H
>>>  #define __LINUX_RESCTRL_TYPES_H
>>>  
>>> +#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
>>> +
>>> +/* rdtgroup.flags */
>>> +#define	RDT_DELETED		1
>>> +
>>> +/* rftype.flags */
>>> +#define RFTYPE_FLAGS_CPUS_LIST	1
>>> +
>>> +/*
>>> + * Define the file type flags for base and info directories.
>>> + */
>>> +#define RFTYPE_INFO			BIT(0)
>>> +#define RFTYPE_BASE			BIT(1)
>>> +#define RFTYPE_CTRL			BIT(4)
>>> +#define RFTYPE_MON			BIT(5)
>>> +#define RFTYPE_TOP			BIT(6)
>>> +#define RFTYPE_RES_CACHE		BIT(8)
>>> +#define RFTYPE_RES_MB			BIT(9)
>>> +#define RFTYPE_DEBUG			BIT(10)
>>> +#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
>>> +#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
>>> +#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
>>> +#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
>>> +#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
>>> +
>>>  /* Reads to Local DRAM Memory */
>>>  #define READS_TO_LOCAL_MEM		BIT(0)
>>>  
>>
>> Not all these new seem to belong in this file. Could you please confirm?
>>
>> For example:
>> Earlier in series it was mentioned that struct rdtgroup is private to the
>> fs so having RDT_DELETED is unexpected as it implies access to struct rdtgroup.
>>
>> CQM_LIMBOCHECK_INTERVAL seems private to the fs code, so too
>> RFTYPE_FLAGS_CPUS_LIST.
>>
>> Reinette
>>
> 
> I'll flag this for James to review.
> 
> These have to be moved out of the x86 private headers, but you're right
> that some of them seem logically private to the resctrl core.
> 
> I guess some of these could move to fs/resctrl/internal.h?

It looks to me that way.

> 
> OTOH, might it be preferable to keep all the flag definitions for a
> given member together for ease of maintenance, even if some are for
> resctrl internal use only?

Indeed, those RFTYPE flags really seem to be fs code but I agree that
architectures' use of RFTYPE_RES_CACHE and RFTYPE_RES_MB does make this
complicated and having these in a central place is reasonable to me.

Reinette
Re: [PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by Dave Martin 1 year, 10 months ago 
On Thu, Apr 11, 2024 at 10:43:55AM -0700, Reinette Chatre wrote:
> Hi Dave,
> 
> On 4/11/2024 7:28 AM, Dave Martin wrote:
> > On Mon, Apr 08, 2024 at 08:42:00PM -0700, Reinette Chatre wrote:
> >> Hi James,
> >>
> >> On 3/21/2024 9:51 AM, James Morse wrote:
> >> ..
> >>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> >>> index 4788bd95dac6..fe0b10b589c0 100644
> >>> --- a/include/linux/resctrl_types.h
> >>> +++ b/include/linux/resctrl_types.h
> >>> @@ -7,6 +7,36 @@
> >>>  #ifndef __LINUX_RESCTRL_TYPES_H
> >>>  #define __LINUX_RESCTRL_TYPES_H
> >>>  
> >>> +#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
> >>> +
> >>> +/* rdtgroup.flags */
> >>> +#define	RDT_DELETED		1
> >>> +
> >>> +/* rftype.flags */
> >>> +#define RFTYPE_FLAGS_CPUS_LIST	1
> >>> +
> >>> +/*
> >>> + * Define the file type flags for base and info directories.
> >>> + */
> >>> +#define RFTYPE_INFO			BIT(0)
> >>> +#define RFTYPE_BASE			BIT(1)
> >>> +#define RFTYPE_CTRL			BIT(4)
> >>> +#define RFTYPE_MON			BIT(5)
> >>> +#define RFTYPE_TOP			BIT(6)
> >>> +#define RFTYPE_RES_CACHE		BIT(8)
> >>> +#define RFTYPE_RES_MB			BIT(9)
> >>> +#define RFTYPE_DEBUG			BIT(10)
> >>> +#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
> >>> +#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
> >>> +#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
> >>> +#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
> >>> +#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
> >>> +
> >>>  /* Reads to Local DRAM Memory */
> >>>  #define READS_TO_LOCAL_MEM		BIT(0)
> >>>  
> >>
> >> Not all these new seem to belong in this file. Could you please confirm?
> >>
> >> For example:
> >> Earlier in series it was mentioned that struct rdtgroup is private to the
> >> fs so having RDT_DELETED is unexpected as it implies access to struct rdtgroup.
> >>
> >> CQM_LIMBOCHECK_INTERVAL seems private to the fs code, so too
> >> RFTYPE_FLAGS_CPUS_LIST.
> >>
> >> Reinette
> >>
> > 
> > I'll flag this for James to review.
> > 
> > These have to be moved out of the x86 private headers, but you're right
> > that some of them seem logically private to the resctrl core.
> > 
> > I guess some of these could move to fs/resctrl/internal.h?
> 
> It looks to me that way.
> 
> > 
> > OTOH, might it be preferable to keep all the flag definitions for a
> > given member together for ease of maintenance, even if some are for
> > resctrl internal use only?
> 
> Indeed, those RFTYPE flags really seem to be fs code but I agree that
> architectures' use of RFTYPE_RES_CACHE and RFTYPE_RES_MB does make this
> complicated and having these in a central place is reasonable to me.
> 
> Reinette

Maybe we could split these into two groups, and clearly comment the ones
that have no user outside resctrl as internal use only?

That's not as clean as removing those definitions from a shared header,
but at least would help document the issue until/unless a better
solution is found...

Cheers
---Dave
Re: [PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by Reinette Chatre 1 year, 9 months ago 
Hi Dave,

On 4/12/2024 9:20 AM, Dave Martin wrote:
> On Thu, Apr 11, 2024 at 10:43:55AM -0700, Reinette Chatre wrote:
>> Hi Dave,
>>
>> On 4/11/2024 7:28 AM, Dave Martin wrote:
>>> On Mon, Apr 08, 2024 at 08:42:00PM -0700, Reinette Chatre wrote:
>>>> Hi James,
>>>>
>>>> On 3/21/2024 9:51 AM, James Morse wrote:
>>>> ..
>>>>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
>>>>> index 4788bd95dac6..fe0b10b589c0 100644
>>>>> --- a/include/linux/resctrl_types.h
>>>>> +++ b/include/linux/resctrl_types.h
>>>>> @@ -7,6 +7,36 @@
>>>>>  #ifndef __LINUX_RESCTRL_TYPES_H
>>>>>  #define __LINUX_RESCTRL_TYPES_H
>>>>>  
>>>>> +#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
>>>>> +
>>>>> +/* rdtgroup.flags */
>>>>> +#define	RDT_DELETED		1
>>>>> +
>>>>> +/* rftype.flags */
>>>>> +#define RFTYPE_FLAGS_CPUS_LIST	1
>>>>> +
>>>>> +/*
>>>>> + * Define the file type flags for base and info directories.
>>>>> + */
>>>>> +#define RFTYPE_INFO			BIT(0)
>>>>> +#define RFTYPE_BASE			BIT(1)
>>>>> +#define RFTYPE_CTRL			BIT(4)
>>>>> +#define RFTYPE_MON			BIT(5)
>>>>> +#define RFTYPE_TOP			BIT(6)
>>>>> +#define RFTYPE_RES_CACHE		BIT(8)
>>>>> +#define RFTYPE_RES_MB			BIT(9)
>>>>> +#define RFTYPE_DEBUG			BIT(10)
>>>>> +#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
>>>>> +#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
>>>>> +#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
>>>>> +#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
>>>>> +#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
>>>>> +
>>>>>  /* Reads to Local DRAM Memory */
>>>>>  #define READS_TO_LOCAL_MEM		BIT(0)
>>>>>  
>>>>
>>>> Not all these new seem to belong in this file. Could you please confirm?
>>>>
>>>> For example:
>>>> Earlier in series it was mentioned that struct rdtgroup is private to the
>>>> fs so having RDT_DELETED is unexpected as it implies access to struct rdtgroup.
>>>>
>>>> CQM_LIMBOCHECK_INTERVAL seems private to the fs code, so too
>>>> RFTYPE_FLAGS_CPUS_LIST.
>>>>
>>>> Reinette
>>>>
>>>
>>> I'll flag this for James to review.
>>>
>>> These have to be moved out of the x86 private headers, but you're right
>>> that some of them seem logically private to the resctrl core.
>>>
>>> I guess some of these could move to fs/resctrl/internal.h?
>>
>> It looks to me that way.
>>
>>>
>>> OTOH, might it be preferable to keep all the flag definitions for a
>>> given member together for ease of maintenance, even if some are for
>>> resctrl internal use only?
>>
>> Indeed, those RFTYPE flags really seem to be fs code but I agree that
>> architectures' use of RFTYPE_RES_CACHE and RFTYPE_RES_MB does make this
>> complicated and having these in a central place is reasonable to me.
>>
>> Reinette
> 
> Maybe we could split these into two groups, and clearly comment the ones
> that have no user outside resctrl as internal use only?

Another option to consider, which I think you hinted about earlier, is
to add an enum that maps to the RFTYPE_RES_CACHE and RFTYPE_RES_MB
flags. Just like, for example, RDTCTRL_GROUP maps to RFTYPE_CTRL.
The new enum can then be used similar to enum rdt_group_type to pick the
appropriate files based on RFTYPE_RES_CACHE or RFTYPE_RES_MB. 

> 
> That's not as clean as removing those definitions from a shared header,
> but at least would help document the issue until/unless a better
> solution is found...
> 
> Cheers
> ---Dave
Re: [PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
Posted by Dave Martin 1 year, 9 months ago 
On Mon, Apr 15, 2024 at 11:03:32AM -0700, Reinette Chatre wrote:
> Hi Dave,
> 
> On 4/12/2024 9:20 AM, Dave Martin wrote:
> > On Thu, Apr 11, 2024 at 10:43:55AM -0700, Reinette Chatre wrote:
> >> Hi Dave,
> >>
> >> On 4/11/2024 7:28 AM, Dave Martin wrote:
> >>> On Mon, Apr 08, 2024 at 08:42:00PM -0700, Reinette Chatre wrote:
> >>>> Hi James,
> >>>>
> >>>> On 3/21/2024 9:51 AM, James Morse wrote:
> >>>> ..
> >>>>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> >>>>> index 4788bd95dac6..fe0b10b589c0 100644
> >>>>> --- a/include/linux/resctrl_types.h
> >>>>> +++ b/include/linux/resctrl_types.h
> >>>>> @@ -7,6 +7,36 @@
> >>>>>  #ifndef __LINUX_RESCTRL_TYPES_H
> >>>>>  #define __LINUX_RESCTRL_TYPES_H
> >>>>>  
> >>>>> +#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
> >>>>> +
> >>>>> +/* rdtgroup.flags */
> >>>>> +#define	RDT_DELETED		1
> >>>>> +
> >>>>> +/* rftype.flags */
> >>>>> +#define RFTYPE_FLAGS_CPUS_LIST	1
> >>>>> +
> >>>>> +/*
> >>>>> + * Define the file type flags for base and info directories.
> >>>>> + */
> >>>>> +#define RFTYPE_INFO			BIT(0)
> >>>>> +#define RFTYPE_BASE			BIT(1)
> >>>>> +#define RFTYPE_CTRL			BIT(4)
> >>>>> +#define RFTYPE_MON			BIT(5)
> >>>>> +#define RFTYPE_TOP			BIT(6)
> >>>>> +#define RFTYPE_RES_CACHE		BIT(8)
> >>>>> +#define RFTYPE_RES_MB			BIT(9)
> >>>>> +#define RFTYPE_DEBUG			BIT(10)
> >>>>> +#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
> >>>>> +#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
> >>>>> +#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
> >>>>> +#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
> >>>>> +#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
> >>>>> +
> >>>>>  /* Reads to Local DRAM Memory */
> >>>>>  #define READS_TO_LOCAL_MEM		BIT(0)
> >>>>>  
> >>>>
> >>>> Not all these new seem to belong in this file. Could you please confirm?
> >>>>
> >>>> For example:
> >>>> Earlier in series it was mentioned that struct rdtgroup is private to the
> >>>> fs so having RDT_DELETED is unexpected as it implies access to struct rdtgroup.
> >>>>
> >>>> CQM_LIMBOCHECK_INTERVAL seems private to the fs code, so too
> >>>> RFTYPE_FLAGS_CPUS_LIST.
> >>>>
> >>>> Reinette
> >>>>
> >>>
> >>> I'll flag this for James to review.
> >>>
> >>> These have to be moved out of the x86 private headers, but you're right
> >>> that some of them seem logically private to the resctrl core.
> >>>
> >>> I guess some of these could move to fs/resctrl/internal.h?
> >>
> >> It looks to me that way.
> >>
> >>>
> >>> OTOH, might it be preferable to keep all the flag definitions for a
> >>> given member together for ease of maintenance, even if some are for
> >>> resctrl internal use only?
> >>
> >> Indeed, those RFTYPE flags really seem to be fs code but I agree that
> >> architectures' use of RFTYPE_RES_CACHE and RFTYPE_RES_MB does make this
> >> complicated and having these in a central place is reasonable to me.
> >>
> >> Reinette
> > 
> > Maybe we could split these into two groups, and clearly comment the ones
> > that have no user outside resctrl as internal use only?
> 
> Another option to consider, which I think you hinted about earlier, is
> to add an enum that maps to the RFTYPE_RES_CACHE and RFTYPE_RES_MB
> flags. Just like, for example, RDTCTRL_GROUP maps to RFTYPE_CTRL.
> The new enum can then be used similar to enum rdt_group_type to pick the
> appropriate files based on RFTYPE_RES_CACHE or RFTYPE_RES_MB. 
> 
> > 
> > That's not as clean as removing those definitions from a shared header,
> > but at least would help document the issue until/unless a better
> > solution is found...
> > 
> > Cheers
> > ---Dave

(Note, I seem to have responded to that suggestion via my reply on patch 3,
rather than here:

"x86/resctrl: Move ctrlval string parsing policy away from the arch code"
https://lore.kernel.org/lkml/Zh6kRMkqVpu0Km4l@e133380.arm.com/ )

Cheers
---Dave

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.