Re: [PATCH v6 28/30] fs/resctrl: Provide interface to create a debugfs info directory

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

Hi Tony,

How about:
 fs/resctrl: Provide interface to create architecture specific debugfs area

On 6/26/25 9:49 AM, Tony Luck wrote:
> Architectures are constrained to just the file interfaces provided by
> the file system for each resource. This does not allow for architecture
> specific debug interfaces.
> 
> Add resctrl_debugfs_mon_info_mkdir() which creates a directory in the

Patch calls it resctrl_debugfs_mon_info_arch_mkdir().

> debugfs file system for a resource. Naming follows the layout of the
> main resctrl hierarchy:
> 
> 	/sys/kernel/debug/resctrl/info/{resource}_MON

Patch creates 
	/sys/kernel/debug/resctrl/info/{resource}_MON/{arch}

Accompanying this change it will be useful to describe how {arch} is
initialized. As a user interface I think it is helpful to connect
that the directory name will match what user can query via "uname -m".

Could you please add a snippet here on how architecture is
expected to use this? There may be some discussion about how archs
could differ in usage of this and mentioning it explicitly in changelog
will help folks see what this enables instead of appearing to sneak
this new feature in.

> 
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  include/linux/resctrl.h |  6 ++++++
>  fs/resctrl/rdtgroup.c   | 24 ++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 35ae24822493..a8ffd9f61c46 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -569,6 +569,12 @@ void resctrl_arch_reset_all_ctrls(struct rdt_resource *r);
>  extern unsigned int resctrl_rmid_realloc_threshold;
>  extern unsigned int resctrl_rmid_realloc_limit;
>  
> +/**
> + * resctrl_debugfs_mon_info_arch_mkdir() - Create a debugfs info directory.
> + * @r:	Resource (must be mon_capable).

Could you please add a snippet here or in function comments of
resctrl_debugfs_mon_info_arch_mkdir() on how architecture is expected to use this?
That is, make it "official" that arch is free to create debugfs files
(and directories) to support its architecture specific debugging 
associated with resource @r.

With this being arch API in include/linux/resctrl.h I think a note on
lifecycle would be useful considering there is no partner
"resctrl_debugfs_mon_info_arch_rmdir()".

As an API to arch it will also be useful to describe what this function returns.
For example, it will be helpful to know that this passes through the
return value of debugfs_create_dir() that arch can use to determine, for example,
whether debugfs is enabled in kernel.

> + */
> +struct dentry *resctrl_debugfs_mon_info_arch_mkdir(struct rdt_resource *r);
> +
>  int resctrl_init(void);
>  void resctrl_exit(void);
>  
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 3d87e6c4c600..511362a67532 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -24,6 +24,7 @@
>  #include <linux/sched/task.h>
>  #include <linux/slab.h>
>  #include <linux/user_namespace.h>
> +#include <linux/utsname.h>
>  
>  #include <uapi/linux/magic.h>
>  
> @@ -4350,6 +4351,29 @@ int resctrl_init(void)
>  	return ret;
>  }
>  
> +/*
> + * Create /sys/kernel/debug/resctrl/info/{r->name}_MON/arch directory

To help make clear that this is not the actual "arch" string: 
arch -> {arch}

> + * by request for architecture to use.
> + */
> +struct dentry *resctrl_debugfs_mon_info_arch_mkdir(struct rdt_resource *r)
> +{
> +	static struct dentry *debugfs_resctrl_info;
> +	struct dentry *moninfodir;
> +	char name[32];
> +
> +	if (!r->mon_capable)
> +		return NULL;
> +
> +	if (!debugfs_resctrl_info)
> +		debugfs_resctrl_info = debugfs_create_dir("info", debugfs_resctrl);
> +
> +	sprintf(name, "%s_MON", r->name);
> +
> +	moninfodir =  debugfs_create_dir(name, debugfs_resctrl_info);

(extra spaces)

> +
> +	return debugfs_create_dir(utsname()->machine, moninfodir);
> +}
> +
>  static bool resctrl_online_domains_exist(void)
>  {
>  	struct rdt_resource *r;

Reinette