To avoid sticky problems in the mpam glue code, move the resctrl
enums into a separate header.
This lets the arch code declare prototypes that use these enums without
creating a loop via asm<->linux resctrl.h The same logic applies to the
monitor-configuration defines, move these too.
The maintainers entry for these headers was missed when resctrl.h
was created. Add a wildcard entry to match both resctrl.h and
resctrl_types.h.
Signed-off-by: James Morse <james.morse@arm.com>
---
internal.h lacks a copyright notice so there is nothing to preserve
when creating a new file...
---
MAINTAINERS | 1 +
arch/x86/kernel/cpu/resctrl/internal.h | 24 ---------
include/linux/resctrl.h | 35 +------------
include/linux/resctrl_types.h | 68 ++++++++++++++++++++++++++
4 files changed, 70 insertions(+), 58 deletions(-)
create mode 100644 include/linux/resctrl_types.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 43b39956694a..5621dd823e79 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18543,6 +18543,7 @@ S: Supported
F: Documentation/arch/x86/resctrl*
F: arch/x86/include/asm/resctrl.h
F: arch/x86/kernel/cpu/resctrl/
+F: include/linux/resctrl*.h
F: tools/testing/selftests/resctrl/
READ-COPY UPDATE (RCU)
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 32ade929ea1b..031948322eab 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -32,30 +32,6 @@
*/
#define MBM_CNTR_WIDTH_OFFSET_MAX (62 - MBM_CNTR_WIDTH_BASE)
-/* Reads to Local DRAM Memory */
-#define READS_TO_LOCAL_MEM BIT(0)
-
-/* Reads to Remote DRAM Memory */
-#define READS_TO_REMOTE_MEM BIT(1)
-
-/* Non-Temporal Writes to Local Memory */
-#define NON_TEMP_WRITE_TO_LOCAL_MEM BIT(2)
-
-/* Non-Temporal Writes to Remote Memory */
-#define NON_TEMP_WRITE_TO_REMOTE_MEM BIT(3)
-
-/* Reads to Local Memory the system identifies as "Slow Memory" */
-#define READS_TO_LOCAL_S_MEM BIT(4)
-
-/* Reads to Remote Memory the system identifies as "Slow Memory" */
-#define READS_TO_REMOTE_S_MEM BIT(5)
-
-/* Dirty Victims to All Types of Memory */
-#define DIRTY_VICTIMS_TO_ALL_MEM BIT(6)
-
-/* Max event bits supported */
-#define MAX_EVT_CONFIG_BITS GENMASK(6, 0)
-
/**
* cpumask_any_housekeeping() - Choose any CPU in @mask, preferring those that
* aren't marked nohz_full
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index c5fcbb524136..b0ee7256e095 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -5,6 +5,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/pid.h>
+#include <linux/resctrl_types.h>
/* CLOSID, RMID value used by the default control group */
#define RESCTRL_RESERVED_CLOSID 0
@@ -24,40 +25,6 @@ int proc_resctrl_show(struct seq_file *m,
/* max value for struct rdt_domain's mbps_val */
#define MBA_MAX_MBPS U32_MAX
-/**
- * enum resctrl_conf_type - The type of configuration.
- * @CDP_NONE: No prioritisation, both code and data are controlled or monitored.
- * @CDP_CODE: Configuration applies to instruction fetches.
- * @CDP_DATA: Configuration applies to reads and writes.
- */
-enum resctrl_conf_type {
- CDP_NONE,
- CDP_CODE,
- CDP_DATA,
-};
-
-enum resctrl_res_level {
- RDT_RESOURCE_L3,
- RDT_RESOURCE_L2,
- RDT_RESOURCE_MBA,
- RDT_RESOURCE_SMBA,
-
- /* Must be the last */
- RDT_NUM_RESOURCES,
-};
-
-#define CDP_NUM_TYPES (CDP_DATA + 1)
-
-/*
- * Event IDs, the values match those used to program IA32_QM_EVTSEL before
- * reading IA32_QM_CTR on RDT systems.
- */
-enum resctrl_event_id {
- QOS_L3_OCCUP_EVENT_ID = 0x01,
- QOS_L3_MBM_TOTAL_EVENT_ID = 0x02,
- QOS_L3_MBM_LOCAL_EVENT_ID = 0x03,
-};
-
/**
* struct resctrl_staged_config - parsed configuration to be applied
* @new_ctrl: new ctrl value to be loaded
diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
new file mode 100644
index 000000000000..4788bd95dac6
--- /dev/null
+++ b/include/linux/resctrl_types.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2024 Arm Ltd.
+ * Based on arch/x86/kernel/cpu/resctrl/internal.h
+ */
+
+#ifndef __LINUX_RESCTRL_TYPES_H
+#define __LINUX_RESCTRL_TYPES_H
+
+/* Reads to Local DRAM Memory */
+#define READS_TO_LOCAL_MEM BIT(0)
+
+/* Reads to Remote DRAM Memory */
+#define READS_TO_REMOTE_MEM BIT(1)
+
+/* Non-Temporal Writes to Local Memory */
+#define NON_TEMP_WRITE_TO_LOCAL_MEM BIT(2)
+
+/* Non-Temporal Writes to Remote Memory */
+#define NON_TEMP_WRITE_TO_REMOTE_MEM BIT(3)
+
+/* Reads to Local Memory the system identifies as "Slow Memory" */
+#define READS_TO_LOCAL_S_MEM BIT(4)
+
+/* Reads to Remote Memory the system identifies as "Slow Memory" */
+#define READS_TO_REMOTE_S_MEM BIT(5)
+
+/* Dirty Victims to All Types of Memory */
+#define DIRTY_VICTIMS_TO_ALL_MEM BIT(6)
+
+/* Max event bits supported */
+#define MAX_EVT_CONFIG_BITS GENMASK(6, 0)
+
+/**
+ * enum resctrl_conf_type - The type of configuration.
+ * @CDP_NONE: No prioritisation, both code and data are controlled or monitored.
+ * @CDP_CODE: Configuration applies to instruction fetches.
+ * @CDP_DATA: Configuration applies to reads and writes.
+ */
+enum resctrl_conf_type {
+ CDP_NONE,
+ CDP_CODE,
+ CDP_DATA,
+};
+
+enum resctrl_res_level {
+ RDT_RESOURCE_L3,
+ RDT_RESOURCE_L2,
+ RDT_RESOURCE_MBA,
+ RDT_RESOURCE_SMBA,
+
+ /* Must be the last */
+ RDT_NUM_RESOURCES,
+};
+
+#define CDP_NUM_TYPES (CDP_DATA + 1)
+
+/*
+ * Event IDs, the values match those used to program IA32_QM_EVTSEL before
+ * reading IA32_QM_CTR on RDT systems.
+ */
+enum resctrl_event_id {
+ QOS_L3_OCCUP_EVENT_ID = 0x01,
+ QOS_L3_MBM_TOTAL_EVENT_ID = 0x02,
+ QOS_L3_MBM_LOCAL_EVENT_ID = 0x03,
+};
+
+#endif /* __LINUX_RESCTRL_TYPES_H */
--
2.39.2Hi James, On 3/21/2024 9:50 AM, James Morse wrote: > To avoid sticky problems in the mpam glue code, move the resctrl > enums into a separate header. Could you please elaborate so that "sticky problems in the mpam glue code" is specific about what problems are avoided? > > This lets the arch code declare prototypes that use these enums without > creating a loop via asm<->linux resctrl.h The same logic applies to the > monitor-configuration defines, move these too. > > The maintainers entry for these headers was missed when resctrl.h > was created. Add a wildcard entry to match both resctrl.h and > resctrl_types.h. > > Signed-off-by: James Morse <james.morse@arm.com> ... > diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h > new file mode 100644 > index 000000000000..4788bd95dac6 > --- /dev/null > +++ b/include/linux/resctrl_types.h > @@ -0,0 +1,68 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2024 Arm Ltd. > + * Based on arch/x86/kernel/cpu/resctrl/internal.h > + */ Could this header please explain how this file is intended to be used? What is it intended to contain? Reinette
On Mon, Apr 08, 2024 at 08:18:00PM -0700, Reinette Chatre wrote: > Hi James, > > On 3/21/2024 9:50 AM, James Morse wrote: > > To avoid sticky problems in the mpam glue code, move the resctrl > > enums into a separate header. > > Could you please elaborate so that "sticky problems in the mpam glue code" is > specific about what problems are avoided? Maybe just delete the the sticky clause, and leave: Move the resctrl enums into a separate header. ...since the next paragraph explains the rationale? > > > > This lets the arch code declare prototypes that use these enums without > > creating a loop via asm<->linux resctrl.h The same logic applies to the > > monitor-configuration defines, move these too. > > > > The maintainers entry for these headers was missed when resctrl.h > > was created. Add a wildcard entry to match both resctrl.h and > > resctrl_types.h. > > > > Signed-off-by: James Morse <james.morse@arm.com> > > .. > > > diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h > > new file mode 100644 > > index 000000000000..4788bd95dac6 > > --- /dev/null > > +++ b/include/linux/resctrl_types.h > > @@ -0,0 +1,68 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Copyright (C) 2024 Arm Ltd. > > + * Based on arch/x86/kernel/cpu/resctrl/internal.h > > + */ > > Could this header please explain how this file is intended to be used? > What is it intended to contain? > > Reinette Maybe something like the following? * Resctrl types and constants needed for inline function definitions in * the arch-specific <asm/resctrl.h> headers. (James might have had other things in mind, but this is what it looks like to me...) Cheers ---Dave
Hi Dave, On 4/12/2024 9:17 AM, Dave Martin wrote: > On Mon, Apr 08, 2024 at 08:18:00PM -0700, Reinette Chatre wrote: >> On 3/21/2024 9:50 AM, James Morse wrote: >>> To avoid sticky problems in the mpam glue code, move the resctrl >>> enums into a separate header. >> >> Could you please elaborate so that "sticky problems in the mpam glue code" is >> specific about what problems are avoided? > > Maybe just delete the the sticky clause, and leave: > > Move the resctrl enums into a separate header. > > ...since the next paragraph explains the rationale? In the x86 area changelogs start with a context paragraph to provide reader with foundation to parse the subsequent problem and solution statements (that are also expected to be in separate paragraphs in that order). >>> This lets the arch code declare prototypes that use these enums without >>> creating a loop via asm<->linux resctrl.h The same logic applies to the >>> monitor-configuration defines, move these too. >>> >>> The maintainers entry for these headers was missed when resctrl.h >>> was created. Add a wildcard entry to match both resctrl.h and >>> resctrl_types.h. >>> >>> Signed-off-by: James Morse <james.morse@arm.com> >> >> .. >> >>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h >>> new file mode 100644 >>> index 000000000000..4788bd95dac6 >>> --- /dev/null >>> +++ b/include/linux/resctrl_types.h >>> @@ -0,0 +1,68 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +/* >>> + * Copyright (C) 2024 Arm Ltd. >>> + * Based on arch/x86/kernel/cpu/resctrl/internal.h >>> + */ >> >> Could this header please explain how this file is intended to be used? >> What is it intended to contain? >> >> Reinette > > Maybe something like the following? > > * Resctrl types and constants needed for inline function definitions in > * the arch-specific <asm/resctrl.h> headers. > ok. Reinette
On Mon, Apr 15, 2024 at 11:03:05AM -0700, Reinette Chatre wrote: > Hi Dave, > > On 4/12/2024 9:17 AM, Dave Martin wrote: > > On Mon, Apr 08, 2024 at 08:18:00PM -0700, Reinette Chatre wrote: > >> On 3/21/2024 9:50 AM, James Morse wrote: > >>> To avoid sticky problems in the mpam glue code, move the resctrl > >>> enums into a separate header. > >> > >> Could you please elaborate so that "sticky problems in the mpam glue code" is > >> specific about what problems are avoided? > > > > Maybe just delete the the sticky clause, and leave: > > > > Move the resctrl enums into a separate header. > > > > ...since the next paragraph explains the rationale? > > In the x86 area changelogs start with a context paragraph to > provide reader with foundation to parse the subsequent problem and > solution statements (that are also expected to be in separate > paragraphs in that order). Acknowledged; I was hoping to avoid a rewrite, but ... > > >>> This lets the arch code declare prototypes that use these enums without > >>> creating a loop via asm<->linux resctrl.h The same logic applies to the > >>> monitor-configuration defines, move these too. [...] OK, how about the following: --8<-- When resctrl is fully factored into core and per-arch code, each arch will need to use some resctrl common definitions in order to define its own specialisations and helpers. Following conventional practice, it would be desirable to put the dependent arch definitions in an <asm/resctrl.h> header that is included by the common <linux/resctrl.h> header. However, this can make it awkward to avoid a circular dependency between <linux/resctrl.h> and the arch header. To avoid solving this issue via conditional inclusion tricks that are likely to be tricky to maintain, move the affected common types and constants into a new header that does not need to depend on <linux/resctrl.h> or on the arch headers. The same logic applies to the monitor-configuration defines, move these too. -->8-- Then: > >>> > >>> The maintainers entry for these headers was missed when resctrl.h > >>> was created. Add a wildcard entry to match both resctrl.h and > >>> resctrl_types.h. > >>> > >>> Signed-off-by: James Morse <james.morse@arm.com> The last paragraph looks like it can stand as-is. Does that look OK? [...] > >>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h > >>> new file mode 100644 > >>> index 000000000000..4788bd95dac6 > >>> --- /dev/null > >>> +++ b/include/linux/resctrl_types.h > >>> @@ -0,0 +1,68 @@ > >>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>> +/* > >>> + * Copyright (C) 2024 Arm Ltd. > >>> + * Based on arch/x86/kernel/cpu/resctrl/internal.h > >>> + */ > >> > >> Could this header please explain how this file is intended to be used? > >> What is it intended to contain? > >> > >> Reinette > > > > Maybe something like the following? > > > > * Resctrl types and constants needed for inline function definitions in > > * the arch-specific <asm/resctrl.h> headers. > > > > ok. > > Reinette > OK, I'll propose to add that. Cheers ---Dave
Hi Dave, On 4/16/2024 9:19 AM, Dave Martin wrote: > On Mon, Apr 15, 2024 at 11:03:05AM -0700, Reinette Chatre wrote: >> Hi Dave, >> >> On 4/12/2024 9:17 AM, Dave Martin wrote: >>> On Mon, Apr 08, 2024 at 08:18:00PM -0700, Reinette Chatre wrote: >>>> On 3/21/2024 9:50 AM, James Morse wrote: >>>>> To avoid sticky problems in the mpam glue code, move the resctrl >>>>> enums into a separate header. >>>> >>>> Could you please elaborate so that "sticky problems in the mpam glue code" is >>>> specific about what problems are avoided? >>> >>> Maybe just delete the the sticky clause, and leave: >>> >>> Move the resctrl enums into a separate header. >>> >>> ...since the next paragraph explains the rationale? >> >> In the x86 area changelogs start with a context paragraph to >> provide reader with foundation to parse the subsequent problem and >> solution statements (that are also expected to be in separate >> paragraphs in that order). > > Acknowledged; I was hoping to avoid a rewrite, but ... >> >>>>> This lets the arch code declare prototypes that use these enums without >>>>> creating a loop via asm<->linux resctrl.h The same logic applies to the >>>>> monitor-configuration defines, move these too. > > [...] > > OK, how about the following: > > --8<-- > > When resctrl is fully factored into core and per-arch code, each arch > will need to use some resctrl common definitions in order to define its > own specialisations and helpers. Following conventional practice, specializations > it would be desirable to put the dependent arch definitions in an > <asm/resctrl.h> header that is included by the common <linux/resctrl.h> > header. However, this can make it awkward to avoid a circular > dependency between <linux/resctrl.h> and the arch header. > > To avoid solving this issue via conditional inclusion tricks that are > likely to be tricky to maintain, move the affected common types and To help with motivation please be specific (somebody may interpret above that it may not be tricky to maintain). So just ... "that are difficult to maintain ...". > constants into a new header that does not need to depend on > <linux/resctrl.h> or on the arch headers. > > The same logic applies to the monitor-configuration defines, move > these too. > > -->8-- > This explains the motivation for this file well, but its contents is not obvious to me and after reading [1] I am more weary of including code before use. Not all of these definitions are needed by the end of this series so there needs to be a good motivation for making things global without any visible user. I do understand that in the next stage of this work we may need to deal with potentially three subsystems when making changes and there is thus a strong motivation for laying a good foundation now. I do not think this should be an excuse to not be diligent in ensuring the changes are required. > Then: > >>>>> >>>>> The maintainers entry for these headers was missed when resctrl.h >>>>> was created. Add a wildcard entry to match both resctrl.h and >>>>> resctrl_types.h. >>>>> >>>>> Signed-off-by: James Morse <james.morse@arm.com> > > The last paragraph looks like it can stand as-is. > > Does that look OK? Yes. Reinette [1] https://lore.kernel.org/lkml/ZhecyLQsGZ9Iv8wU@gmail.com/
On Wed, Apr 17, 2024 at 10:15:57PM -0700, Reinette Chatre wrote: > Hi Dave, > > On 4/16/2024 9:19 AM, Dave Martin wrote: > > On Mon, Apr 15, 2024 at 11:03:05AM -0700, Reinette Chatre wrote: > >> Hi Dave, > >> > >> On 4/12/2024 9:17 AM, Dave Martin wrote: > >>> On Mon, Apr 08, 2024 at 08:18:00PM -0700, Reinette Chatre wrote: > >>>> On 3/21/2024 9:50 AM, James Morse wrote: > >>>>> To avoid sticky problems in the mpam glue code, move the resctrl > >>>>> enums into a separate header. > >>>> > >>>> Could you please elaborate so that "sticky problems in the mpam glue code" is > >>>> specific about what problems are avoided? > >>> > >>> Maybe just delete the the sticky clause, and leave: > >>> > >>> Move the resctrl enums into a separate header. > >>> > >>> ...since the next paragraph explains the rationale? > >> > >> In the x86 area changelogs start with a context paragraph to > >> provide reader with foundation to parse the subsequent problem and > >> solution statements (that are also expected to be in separate > >> paragraphs in that order). > > > > Acknowledged; I was hoping to avoid a rewrite, but ... > >> > >>>>> This lets the arch code declare prototypes that use these enums without > >>>>> creating a loop via asm<->linux resctrl.h The same logic applies to the > >>>>> monitor-configuration defines, move these too. > > > > [...] > > > > OK, how about the following: > > > > --8<-- > > > > When resctrl is fully factored into core and per-arch code, each arch > > will need to use some resctrl common definitions in order to define its > > own specialisations and helpers. Following conventional practice, > > specializations Debatable, but OK, fine. > > it would be desirable to put the dependent arch definitions in an > > <asm/resctrl.h> header that is included by the common <linux/resctrl.h> > > header. However, this can make it awkward to avoid a circular > > dependency between <linux/resctrl.h> and the arch header. > > > > To avoid solving this issue via conditional inclusion tricks that are > > likely to be tricky to maintain, move the affected common types and > > To help with motivation please be specific (somebody may interpret above > that it may not be tricky to maintain). So just ... "that are difficult > to maintain ...". Rather than the text encouraging questions about whether there are reasonable alternative approaches, perhaps this can just be, simply: "To avoid such dependencies, move the affected types into a new header [...]" ? > > > constants into a new header that does not need to depend on > > <linux/resctrl.h> or on the arch headers. > > > > The same logic applies to the monitor-configuration defines, move > > these too. > > > > -->8-- > > > > This explains the motivation for this file well, but its contents > is not obvious to me and after reading [1] I am more weary of including > code before use. Not all of these definitions are needed > by the end of this series so there needs to be a good motivation for > making things global without any visible user. That's fair. I guess we need to review the contents of this header and make sure that everything that's here really should be here. However, this is not user ABI and there are only 1.5 users of this interface (given that MPAM is not yet merged). So, the penalty for not getting this quite right and fixing it later seems low. If you agree that adding this header is appropriate, are you OK with some post-merge cleanup, or do you think it's essential to sanitise this fully up-front? > > I do understand that in the next stage of this work we may need to deal > with potentially three subsystems when making changes and there is thus > a strong motivation for laying a good foundation now. I do not think this > should be an excuse to not be diligent in ensuring the changes are > required. > > > Then: > > > >>>>> > >>>>> The maintainers entry for these headers was missed when resctrl.h > >>>>> was created. Add a wildcard entry to match both resctrl.h and > >>>>> resctrl_types.h. > >>>>> > >>>>> Signed-off-by: James Morse <james.morse@arm.com> > > > > The last paragraph looks like it can stand as-is. > > > > Does that look OK? > > Yes. > > Reinette > > [1] https://lore.kernel.org/lkml/ZhecyLQsGZ9Iv8wU@gmail.com/ > Cheers ---Dave
Hi Dave, On 4/18/2024 8:25 AM, Dave Martin wrote: > On Wed, Apr 17, 2024 at 10:15:57PM -0700, Reinette Chatre wrote: >> On 4/16/2024 9:19 AM, Dave Martin wrote: >>> On Mon, Apr 15, 2024 at 11:03:05AM -0700, Reinette Chatre wrote: >>>> On 4/12/2024 9:17 AM, Dave Martin wrote: >>>>> On Mon, Apr 08, 2024 at 08:18:00PM -0700, Reinette Chatre >>>>> wrote: >>>>>> On 3/21/2024 9:50 AM, James Morse wrote: >>>>>>> To avoid sticky problems in the mpam glue code, move the >>>>>>> resctrl enums into a separate header. >>>>>> >>>>>> Could you please elaborate so that "sticky problems in the >>>>>> mpam glue code" is specific about what problems are >>>>>> avoided? >>>>> >>>>> Maybe just delete the the sticky clause, and leave: >>>>> >>>>> Move the resctrl enums into a separate header. >>>>> >>>>> ...since the next paragraph explains the rationale? >>>> >>>> In the x86 area changelogs start with a context paragraph to >>>> provide reader with foundation to parse the subsequent problem >>>> and solution statements (that are also expected to be in >>>> separate paragraphs in that order). >>> >>> Acknowledged; I was hoping to avoid a rewrite, but ... >>>> >>>>>>> This lets the arch code declare prototypes that use these >>>>>>> enums without creating a loop via asm<->linux resctrl.h >>>>>>> The same logic applies to the monitor-configuration >>>>>>> defines, move these too. >>> >>> [...] >>> >>> OK, how about the following: >>> >>> --8<-- >>> >>> When resctrl is fully factored into core and per-arch code, each >>> arch will need to use some resctrl common definitions in order to >>> define its own specialisations and helpers. Following >>> conventional practice, >> >> specializations > > Debatable, but OK, fine. ah British spelling, apologies. > >>> it would be desirable to put the dependent arch definitions in >>> an <asm/resctrl.h> header that is included by the common >>> <linux/resctrl.h> header. However, this can make it awkward to >>> avoid a circular dependency between <linux/resctrl.h> and the >>> arch header. >>> >>> To avoid solving this issue via conditional inclusion tricks that >>> are likely to be tricky to maintain, move the affected common >>> types and >> >> To help with motivation please be specific (somebody may interpret >> above that it may not be tricky to maintain). So just ... "that are >> difficult to maintain ...". > > Rather than the text encouraging questions about whether there are > reasonable alternative approaches, perhaps this can just be, simply: > > "To avoid such dependencies, move the affected types into a new > header [...]" > > ? Sure. > >> >>> constants into a new header that does not need to depend on >>> <linux/resctrl.h> or on the arch headers. >>> >>> The same logic applies to the monitor-configuration defines, >>> move these too. >>> >>> -->8-- >>> >> >> This explains the motivation for this file well, but its contents >> is not obvious to me and after reading [1] I am more weary of >> including code before use. Not all of these definitions are needed >> by the end of this series so there needs to be a good motivation >> for making things global without any visible user. > > That's fair. I guess we need to review the contents of this header > and make sure that everything that's here really should be here. > > However, this is not user ABI and there are only 1.5 users of this > interface (given that MPAM is not yet merged). So, the penalty for > not getting this quite right and fixing it later seems low. > > If you agree that adding this header is appropriate, are you OK with > some post-merge cleanup, or do you think it's essential to sanitise > this fully up-front? > I think you may have sent this before your response to patch #17 where you are talking about keeping some definitions x86 specific until their usage is clear. I understand this is not user ABI and as I also stated previously I recognize that these changes are easier now than later when changes need to cross two subsystems. I do not think the goal should be to have the perfect header file but I would like to understand why each definition in it needs to be global. Unfortunately, based on learnings during the four year history of this work, I do not have confidence that there will be post-merge cleanup. Reinette
© 2016 - 2026 Red Hat, Inc.