Re: [PATCH v1 29/31] fs/resctrl: Add boiler plate for external resctrl code

[Reinette Chatre <reinette.chatre@intel.com>]

Hi James,

On 3/21/2024 9:51 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.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  MAINTAINERS              |  1 +
>  arch/Kconfig             |  8 ++++++++
>  arch/x86/Kconfig         | 10 +++-------
>  fs/Kconfig               |  1 +
>  fs/Makefile              |  1 +
>  fs/resctrl/Kconfig       | 23 +++++++++++++++++++++++
>  fs/resctrl/Makefile      |  3 +++
>  fs/resctrl/ctrlmondata.c |  0
>  fs/resctrl/internal.h    |  0
>  fs/resctrl/monitor.c     |  0
>  fs/resctrl/psuedo_lock.c |  0
>  fs/resctrl/rdtgroup.c    |  0
>  include/linux/resctrl.h  |  4 ++++
>  13 files changed, 44 insertions(+), 7 deletions(-)
>  create mode 100644 fs/resctrl/Kconfig
>  create mode 100644 fs/resctrl/Makefile
>  create mode 100644 fs/resctrl/ctrlmondata.c
>  create mode 100644 fs/resctrl/internal.h
>  create mode 100644 fs/resctrl/monitor.c
>  create mode 100644 fs/resctrl/psuedo_lock.c
>  create mode 100644 fs/resctrl/rdtgroup.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5621dd823e79..c49090e9c777 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:	fs/resctrl/
>  F:	include/linux/resctrl*.h
>  F:	tools/testing/selftests/resctrl/
>  
> diff --git a/arch/Kconfig b/arch/Kconfig
> index fd18b7db2c77..131d874d6738 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1406,6 +1406,14 @@ config STRICT_MODULE_RWX
>  config ARCH_HAS_PHYS_TO_DMA
>  	bool
>  
> +config ARCH_HAS_CPU_RESCTRL
> +	bool
> +	help
> +	  The 'resctrl' filesystem allows CPU controls of shared resources
> +	  such as caches and memory bandwidth to be configured. An architecture
> +	  selects this if it provides the arch-specific hooks for the filesystem
> +	  and needs the per-task CLOSID/RMID properties.

Should it mention monitoring capabilities?

> +
>  config HAVE_ARCH_COMPILER_H
>  	bool
>  	help
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index e071e564452e..cb043543f088 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -479,8 +479,10 @@ config GOLDFISH
>  config X86_CPU_RESCTRL
>  	bool "x86 CPU resource control support"
>  	depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
> +	depends on MISC_FILESYSTEMS
>  	select KERNFS

Do both X86_CPU_RESCTRL and RESCTRL_FS need to select KERNFS?

> -	select PROC_CPU_RESCTRL		if PROC_FS
> +	select ARCH_HAS_CPU_RESCTRL
> +	select RESCTRL_FS
>  	select RESCTRL_FS_PSEUDO_LOCK
>  	help
>  	  Enable x86 CPU resource control support.
> @@ -507,12 +509,6 @@ config X86_FRED
>  	  ring transitions and exception/interrupt handling if the
>  	  system supports.
>  
> -config RESCTRL_FS_PSEUDO_LOCK
> -	bool
> -	help
> -	  Software mechanism to pin data in a cache portion using
> -	  micro-architecture specific knowledge.
> -
>  if X86_32
>  config X86_BIGSMP
>  	bool "Support for big SMP systems with more than 8 CPUs"
> diff --git a/fs/Kconfig b/fs/Kconfig
> index a46b0cbc4d8f..d8a36383b6dc 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -331,6 +331,7 @@ source "fs/omfs/Kconfig"
>  source "fs/hpfs/Kconfig"
>  source "fs/qnx4/Kconfig"
>  source "fs/qnx6/Kconfig"
> +source "fs/resctrl/Kconfig"
>  source "fs/romfs/Kconfig"
>  source "fs/pstore/Kconfig"
>  source "fs/sysv/Kconfig"
> diff --git a/fs/Makefile b/fs/Makefile
> index 6ecc9b0a53f2..da6e2d028722 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -129,3 +129,4 @@ obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
>  obj-$(CONFIG_EROFS_FS)		+= erofs/
>  obj-$(CONFIG_VBOXSF_FS)		+= vboxsf/
>  obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
> +obj-$(CONFIG_RESCTRL_FS)	+= resctrl/
> diff --git a/fs/resctrl/Kconfig b/fs/resctrl/Kconfig
> new file mode 100644
> index 000000000000..36a1ddbe6c21
> --- /dev/null
> +++ b/fs/resctrl/Kconfig

Could you please review the contents of this file for
appropriate line length and consistent tab usage?

> @@ -0,0 +1,23 @@
> +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
> +	  Resctrl is a filesystem interface
> +	  to control allocation and
> +	  monitoring of system resources
> +	  used by the CPUs.
> +
> +config RESCTRL_FS_PSEUDO_LOCK
> +	bool
> +	help
> +          Software mechanism to pin data in a cache portion using
> +          micro-architecture specific knowledge.
> +
> +config RESCTRL_RMID_DEPENDS_ON_CLOSID
> +	bool
> +	help
> +	  Enable by the architecture when the RMID values depend on the CLOSID.
> +	  This causes the closid allocator to search for CLOSID with clean
> +	  RMID.
> diff --git a/fs/resctrl/Makefile b/fs/resctrl/Makefile
> new file mode 100644
> index 000000000000..10fcfb0fdb10
> --- /dev/null
> +++ b/fs/resctrl/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_RESCTRL_FS)		+= rdtgroup.o ctrlmondata.o monitor.o
> +obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= psuedo_lock.o
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/fs/resctrl/psuedo_lock.c b/fs/resctrl/psuedo_lock.c

pseudo_lock.c

> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 5da55e58f229..f786ffceeda3 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -8,6 +8,10 @@
>  #include <linux/pid.h>
>  #include <linux/resctrl_types.h>
>  
> +#ifdef CONFIG_ARCH_HAS_CPU_RESCTRL
> +#include <asm/resctrl.h>
> +#endif
> +
>  /* CLOSID, RMID value used by the default control group */
>  #define RESCTRL_RESERVED_CLOSID		0
>  #define RESCTRL_RESERVED_RMID		0

Reinette