[PATCH] x86: avoid confusion over the new RESCTRL config prompt

[PATCH] x86: avoid confusion over the new RESCTRL config prompt

[Johannes Weiner]

"Resource Control" is a very broad term for this CPU feature, and a
term that is also associated with containers, cgroups etc. This can
easily cause confusion.

Make the user prompt more specific.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 arch/x86/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

As somebody who works in "resource control", this prompt in oldconfig
certainly had me go wtf. Can we do something more specific?

Not insisting on this name, and I haven't renamed all the config
symbols yet, but we should probably fix this before 5.0 is released.

Thanks!

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 26387c7bf305..426677b759c0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -447,11 +447,11 @@ config RETPOLINE
 	  support for full protection. The kernel may run slower.
 
 config X86_RESCTRL
-	bool "Resource Control support"
+	bool "x86 cache control support"
 	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	select KERNFS
 	help
-	  Enable Resource Control support.
+	  Enable x86 cache control support.
 
 	  Provide support for the allocation and monitoring of system resources
 	  usage by the CPU.
-- 
2.20.1

Re: [PATCH] x86: avoid confusion over the new RESCTRL config prompt

[Borislav Petkov]

On Tue, Jan 29, 2019 at 05:52:18PM -0500, Johannes Weiner wrote:
> "Resource Control" is a very broad term for this CPU feature, and a
> term that is also associated with containers, cgroups etc. This can
> easily cause confusion.
> 
> Make the user prompt more specific.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
>  arch/x86/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> As somebody who works in "resource control", this prompt in oldconfig
> certainly had me go wtf. Can we do something more specific?

Well, since this is an interface to a hardware feature, we could call it

Hardware Resource Control

for example. I.e., HW_RESCTRL.

Linus had another suggestion - CPU_RESCTRL - which sounds ok to me too.

> Not insisting on this name, and I haven't renamed all the config
> symbols yet, but we should probably fix this before 5.0 is released.
> 
> Thanks!
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 26387c7bf305..426677b759c0 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -447,11 +447,11 @@ config RETPOLINE
>  	  support for full protection. The kernel may run slower.
>  
>  config X86_RESCTRL
> -	bool "Resource Control support"
> +	bool "x86 cache control support"

Except that it is not only cache but memory (bandwidth) control too. So I guess

	bool "CPU Resource Control support"

or so.

Let the bikeshedding begin!

:-)

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH] x86: avoid confusion over the new RESCTRL config prompt

[Luck, Tony]

On Wed, Jan 30, 2019 at 12:08:45AM +0100, Borislav Petkov wrote:
> On Tue, Jan 29, 2019 at 05:52:18PM -0500, Johannes Weiner wrote:
> >  config X86_RESCTRL
> > -	bool "Resource Control support"
> > +	bool "x86 cache control support"
> 
> Except that it is not only cache but memory (bandwidth) control too. So I guess
> 
> 	bool "CPU Resource Control support"
> 
> or so.
> 
> Let the bikeshedding begin!

I'll just jump on the bandwagon and vote for "CPU Resource Control support".

You are exactly right that this isn't just about cache.

-Tony

Re: [PATCH] x86: avoid confusion over the new RESCTRL config prompt

[Johannes Weiner]

On Wed, Jan 30, 2019 at 12:08:45AM +0100, Borislav Petkov wrote:
> On Tue, Jan 29, 2019 at 05:52:18PM -0500, Johannes Weiner wrote:
> > "Resource Control" is a very broad term for this CPU feature, and a
> > term that is also associated with containers, cgroups etc. This can
> > easily cause confusion.
> > 
> > Make the user prompt more specific.
> > 
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > ---
> >  arch/x86/Kconfig | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > As somebody who works in "resource control", this prompt in oldconfig
> > certainly had me go wtf. Can we do something more specific?
> 
> Well, since this is an interface to a hardware feature, we could call it
> 
> Hardware Resource Control
> 
> for example. I.e., HW_RESCTRL.
> 
> Linus had another suggestion - CPU_RESCTRL - which sounds ok to me too.
> 
> > Not insisting on this name, and I haven't renamed all the config
> > symbols yet, but we should probably fix this before 5.0 is released.
> > 
> > Thanks!
> > 
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 26387c7bf305..426677b759c0 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -447,11 +447,11 @@ config RETPOLINE
> >  	  support for full protection. The kernel may run slower.
> >  
> >  config X86_RESCTRL
> > -	bool "Resource Control support"
> > +	bool "x86 cache control support"
> 
> Except that it is not only cache but memory (bandwidth) control too. So I guess
> 
> 	bool "CPU Resource Control support"

This is still awefully close to the cgroup CPU resource
controller. Since it's more hardware-specific, and the config symbol
also has the x86 in it, how about "x86 CPU resource control support"?

---

From 2a24f6e30ed9dbc0abb96c73ec0e205e07034383 Mon Sep 17 00:00:00 2001
From: Johannes Weiner <hannes@cmpxchg.org>
Date: Tue, 29 Jan 2019 17:44:36 -0500
Subject: [PATCH] x86: avoid confusion over the new RESCTRL config prompt

"Resource Control" is a very broad term for this CPU feature, and a
term that is also associated with containers, cgroups etc. This can
easily cause confusion.

Make the user prompt more specific.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 arch/x86/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 26387c7bf305..ac432379df11 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -447,11 +447,11 @@ config RETPOLINE
 	  support for full protection. The kernel may run slower.
 
 config X86_RESCTRL
-	bool "Resource Control support"
+	bool "x86 CPU resource control support"
 	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	select KERNFS
 	help
-	  Enable Resource Control support.
+	  Enable x86 CPU resource control support.
 
 	  Provide support for the allocation and monitoring of system resources
 	  usage by the CPU.
-- 
2.20.1

Re: [PATCH] x86: avoid confusion over the new RESCTRL config prompt

[Borislav Petkov]

On Wed, Jan 30, 2019 at 11:46:26AM -0500, Johannes Weiner wrote:
> This is still awefully close to the cgroup CPU resource
> controller. Since it's more hardware-specific, and the config symbol
> also has the x86 in it, how about "x86 CPU resource control support"?

Fine by me.

What about

s/X86_RESCTRL/X86_CPU_RESCTRL/g

?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH] x86: avoid confusion over the new RESCTRL config prompt

[Johannes Weiner]

On Wed, Jan 30, 2019 at 06:19:33PM +0100, Borislav Petkov wrote:
> On Wed, Jan 30, 2019 at 11:46:26AM -0500, Johannes Weiner wrote:
> > This is still awefully close to the cgroup CPU resource
> > controller. Since it's more hardware-specific, and the config symbol
> > also has the x86 in it, how about "x86 CPU resource control support"?
> 
> Fine by me.
> 
> What about
> 
> s/X86_RESCTRL/X86_CPU_RESCTRL/g

Good idea.

Tony, Babu, that look okay to you guys as well?

---

From 9ca5fa615f9f5d270c141266c454059a3de2cd39 Mon Sep 17 00:00:00 2001
From: Johannes Weiner <hannes@cmpxchg.org>
Date: Tue, 29 Jan 2019 17:44:36 -0500
Subject: [PATCH] x86: avoid confusion over the new X86_RESCTRL config

"Resource Control" is a very broad term for this CPU feature, and a
term that is also associated with containers, cgroups etc. This can
easily cause confusion.

Make the user prompt more specific. Match the config symbol name.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 Documentation/x86/resctrl_ui.txt     | 2 +-
 arch/x86/Kconfig                     | 6 +++---
 arch/x86/include/asm/resctrl_sched.h | 4 ++--
 arch/x86/kernel/cpu/Makefile         | 2 +-
 arch/x86/kernel/cpu/resctrl/Makefile | 4 ++--
 include/linux/sched.h                | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/x86/resctrl_ui.txt b/Documentation/x86/resctrl_ui.txt
index e8e8d14d3c4e..c1f95b59e14d 100644
--- a/Documentation/x86/resctrl_ui.txt
+++ b/Documentation/x86/resctrl_ui.txt
@@ -9,7 +9,7 @@ Fenghua Yu <fenghua.yu@intel.com>
 Tony Luck <tony.luck@intel.com>
 Vikas Shivappa <vikas.shivappa@intel.com>
 
-This feature is enabled by the CONFIG_X86_RESCTRL and the x86 /proc/cpuinfo
+This feature is enabled by the CONFIG_X86_CPU_RESCTRL and the x86 /proc/cpuinfo
 flag bits:
 RDT (Resource Director Technology) Allocation - "rdt_a"
 CAT (Cache Allocation Technology) - "cat_l3", "cat_l2"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 26387c7bf305..68261430fe6e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -446,12 +446,12 @@ config RETPOLINE
 	  branches. Requires a compiler with -mindirect-branch=thunk-extern
 	  support for full protection. The kernel may run slower.
 
-config X86_RESCTRL
-	bool "Resource Control support"
+config X86_CPU_RESCTRL
+	bool "x86 CPU resource control support"
 	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	select KERNFS
 	help
-	  Enable Resource Control support.
+	  Enable x86 CPU resource control support.
 
 	  Provide support for the allocation and monitoring of system resources
 	  usage by the CPU.
diff --git a/arch/x86/include/asm/resctrl_sched.h b/arch/x86/include/asm/resctrl_sched.h
index 40ebddde6ac2..f6b7fe2833cc 100644
--- a/arch/x86/include/asm/resctrl_sched.h
+++ b/arch/x86/include/asm/resctrl_sched.h
@@ -2,7 +2,7 @@
 #ifndef _ASM_X86_RESCTRL_SCHED_H
 #define _ASM_X86_RESCTRL_SCHED_H
 
-#ifdef CONFIG_X86_RESCTRL
+#ifdef CONFIG_X86_CPU_RESCTRL
 
 #include <linux/sched.h>
 #include <linux/jump_label.h>
@@ -88,6 +88,6 @@ static inline void resctrl_sched_in(void)
 
 static inline void resctrl_sched_in(void) {}
 
-#endif /* CONFIG_X86_RESCTRL */
+#endif /* CONFIG_X86_CPU_RESCTRL */
 
 #endif /* _ASM_X86_RESCTRL_SCHED_H */
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index b6fa0869f7aa..cfd24f9f7614 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -39,7 +39,7 @@ obj-$(CONFIG_CPU_SUP_UMC_32)		+= umc.o
 obj-$(CONFIG_X86_MCE)			+= mce/
 obj-$(CONFIG_MTRR)			+= mtrr/
 obj-$(CONFIG_MICROCODE)			+= microcode/
-obj-$(CONFIG_X86_RESCTRL)		+= resctrl/
+obj-$(CONFIG_X86_CPU_RESCTRL)		+= resctrl/
 
 obj-$(CONFIG_X86_LOCAL_APIC)		+= perfctr-watchdog.o
 
diff --git a/arch/x86/kernel/cpu/resctrl/Makefile b/arch/x86/kernel/cpu/resctrl/Makefile
index 1cabe6fd8e11..4a06c37b9cf1 100644
--- a/arch/x86/kernel/cpu/resctrl/Makefile
+++ b/arch/x86/kernel/cpu/resctrl/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_X86_RESCTRL)	+= core.o rdtgroup.o monitor.o
-obj-$(CONFIG_X86_RESCTRL)	+= ctrlmondata.o pseudo_lock.o
+obj-$(CONFIG_X86_CPU_RESCTRL)	+= core.o rdtgroup.o monitor.o
+obj-$(CONFIG_X86_CPU_RESCTRL)	+= ctrlmondata.o pseudo_lock.o
 CFLAGS_pseudo_lock.o = -I$(src)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d2f90fa92468..bba3afb4e9bf 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -995,7 +995,7 @@ struct task_struct {
 	/* cg_list protected by css_set_lock and tsk->alloc_lock: */
 	struct list_head		cg_list;
 #endif
-#ifdef CONFIG_X86_RESCTRL
+#ifdef CONFIG_X86_CPU_RESCTRL
 	u32				closid;
 	u32				rmid;
 #endif
-- 
2.20.1

RE: [PATCH] x86: avoid confusion over the new RESCTRL config prompt

[Luck, Tony]

>> What about
>> 
>> s/X86_RESCTRL/X86_CPU_RESCTRL/g
>
> Good idea.
>
> Tony, Babu, that look okay to you guys as well?

For now.  But very soon we will also have ARM_CPU_RESCTRL, and some of
this code will become generic.  Will we need an arch-independent name for the
bits of code shared by arm and x86?  

-Tony

Re: [PATCH] x86: avoid confusion over the new RESCTRL config prompt

[Borislav Petkov]

On Fri, Feb 01, 2019 at 07:18:35PM +0000, Luck, Tony wrote:
> For now.  But very soon we will also have ARM_CPU_RESCTRL, and some of
> this code will become generic.  Will we need an arch-independent name for the
> bits of code shared by arm and x86?

Yes. The idea still is to have the arch-specific stuff under
X86_CPU_RESCTRL and ARM_CPU_RESCTRL respectively, and the arch-agnostic
pieces under CPU_RESCTRL.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

[tip:x86/urgent] x86/resctrl: Avoid confusion over the new X86_RESCTRL config

[tip-bot for Johannes Weiner]

Commit-ID:  e6d429313ea5c776d2e76b4494df69102e6b7115
Gitweb:     https://git.kernel.org/tip/e6d429313ea5c776d2e76b4494df69102e6b7115
Author:     Johannes Weiner <hannes@cmpxchg.org>
AuthorDate: Tue, 29 Jan 2019 17:44:36 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Sat, 2 Feb 2019 10:34:52 +0100

x86/resctrl: Avoid confusion over the new X86_RESCTRL config

"Resource Control" is a very broad term for this CPU feature, and a term
that is also associated with containers, cgroups etc. This can easily
cause confusion.

Make the user prompt more specific. Match the config symbol name.

 [ bp: In the future, the corresponding ARM arch-specific code will be
   under ARM_CPU_RESCTRL and the arch-agnostic bits will be carved out
   under the CPU_RESCTRL umbrella symbol. ]

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Babu Moger <Babu.Moger@amd.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Morse <james.morse@arm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: linux-doc@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Pu Wen <puwen@hygon.cn>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20190130195621.GA30653@cmpxchg.org
---
 Documentation/x86/resctrl_ui.txt     | 2 +-
 arch/x86/Kconfig                     | 6 +++---
 arch/x86/include/asm/resctrl_sched.h | 4 ++--
 arch/x86/kernel/cpu/Makefile         | 2 +-
 arch/x86/kernel/cpu/resctrl/Makefile | 4 ++--
 include/linux/sched.h                | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/x86/resctrl_ui.txt b/Documentation/x86/resctrl_ui.txt
index e8e8d14d3c4e..c1f95b59e14d 100644
--- a/Documentation/x86/resctrl_ui.txt
+++ b/Documentation/x86/resctrl_ui.txt
@@ -9,7 +9,7 @@ Fenghua Yu <fenghua.yu@intel.com>
 Tony Luck <tony.luck@intel.com>
 Vikas Shivappa <vikas.shivappa@intel.com>
 
-This feature is enabled by the CONFIG_X86_RESCTRL and the x86 /proc/cpuinfo
+This feature is enabled by the CONFIG_X86_CPU_RESCTRL and the x86 /proc/cpuinfo
 flag bits:
 RDT (Resource Director Technology) Allocation - "rdt_a"
 CAT (Cache Allocation Technology) - "cat_l3", "cat_l2"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 26387c7bf305..68261430fe6e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -446,12 +446,12 @@ config RETPOLINE
 	  branches. Requires a compiler with -mindirect-branch=thunk-extern
 	  support for full protection. The kernel may run slower.
 
-config X86_RESCTRL
-	bool "Resource Control support"
+config X86_CPU_RESCTRL
+	bool "x86 CPU resource control support"
 	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
 	select KERNFS
 	help
-	  Enable Resource Control support.
+	  Enable x86 CPU resource control support.
 
 	  Provide support for the allocation and monitoring of system resources
 	  usage by the CPU.
diff --git a/arch/x86/include/asm/resctrl_sched.h b/arch/x86/include/asm/resctrl_sched.h
index 40ebddde6ac2..f6b7fe2833cc 100644
--- a/arch/x86/include/asm/resctrl_sched.h
+++ b/arch/x86/include/asm/resctrl_sched.h
@@ -2,7 +2,7 @@
 #ifndef _ASM_X86_RESCTRL_SCHED_H
 #define _ASM_X86_RESCTRL_SCHED_H
 
-#ifdef CONFIG_X86_RESCTRL
+#ifdef CONFIG_X86_CPU_RESCTRL
 
 #include <linux/sched.h>
 #include <linux/jump_label.h>
@@ -88,6 +88,6 @@ static inline void resctrl_sched_in(void)
 
 static inline void resctrl_sched_in(void) {}
 
-#endif /* CONFIG_X86_RESCTRL */
+#endif /* CONFIG_X86_CPU_RESCTRL */
 
 #endif /* _ASM_X86_RESCTRL_SCHED_H */
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index b6fa0869f7aa..cfd24f9f7614 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -39,7 +39,7 @@ obj-$(CONFIG_CPU_SUP_UMC_32)		+= umc.o
 obj-$(CONFIG_X86_MCE)			+= mce/
 obj-$(CONFIG_MTRR)			+= mtrr/
 obj-$(CONFIG_MICROCODE)			+= microcode/
-obj-$(CONFIG_X86_RESCTRL)		+= resctrl/
+obj-$(CONFIG_X86_CPU_RESCTRL)		+= resctrl/
 
 obj-$(CONFIG_X86_LOCAL_APIC)		+= perfctr-watchdog.o
 
diff --git a/arch/x86/kernel/cpu/resctrl/Makefile b/arch/x86/kernel/cpu/resctrl/Makefile
index 1cabe6fd8e11..4a06c37b9cf1 100644
--- a/arch/x86/kernel/cpu/resctrl/Makefile
+++ b/arch/x86/kernel/cpu/resctrl/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_X86_RESCTRL)	+= core.o rdtgroup.o monitor.o
-obj-$(CONFIG_X86_RESCTRL)	+= ctrlmondata.o pseudo_lock.o
+obj-$(CONFIG_X86_CPU_RESCTRL)	+= core.o rdtgroup.o monitor.o
+obj-$(CONFIG_X86_CPU_RESCTRL)	+= ctrlmondata.o pseudo_lock.o
 CFLAGS_pseudo_lock.o = -I$(src)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 224666226e87..8c328b14c424 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -995,7 +995,7 @@ struct task_struct {
 	/* cg_list protected by css_set_lock and tsk->alloc_lock: */
 	struct list_head		cg_list;
 #endif
-#ifdef CONFIG_X86_RESCTRL
+#ifdef CONFIG_X86_CPU_RESCTRL
 	u32				closid;
 	u32				rmid;
 #endif