linux-kernel - Re: [PATCH v16 27/34] fs/resctrl: Introduce mbm_assign_on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f653a942-b2e7-45d2-b5fb-458b022a25bd@intel.com>
Date: Wed, 30 Jul 2025 13:08:18 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Babu Moger <babu.moger@....com>, <corbet@....net>, <tony.luck@...el.com>,
	<james.morse@....com>, <tglx@...utronix.de>, <mingo@...hat.com>,
	<bp@...en8.de>, <dave.hansen@...ux.intel.com>
CC: <Dave.Martin@....com>, <x86@...nel.org>, <hpa@...or.com>,
	<akpm@...ux-foundation.org>, <paulmck@...nel.org>, <rostedt@...dmis.org>,
	<Neeraj.Upadhyay@....com>, <david@...hat.com>, <arnd@...db.de>,
	<fvdl@...gle.com>, <seanjc@...gle.com>, <jpoimboe@...nel.org>,
	<pawan.kumar.gupta@...ux.intel.com>, <xin@...or.com>,
	<manali.shukla@....com>, <tao1.su@...ux.intel.com>, <sohil.mehta@...el.com>,
	<kai.huang@...el.com>, <xiaoyao.li@...el.com>, <peterz@...radead.org>,
	<xin3.li@...el.com>, <kan.liang@...ux.intel.com>,
	<mario.limonciello@....com>, <thomas.lendacky@....com>, <perry.yuan@....com>,
	<gautham.shenoy@....com>, <chang.seok.bae@...el.com>,
	<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<peternewman@...gle.com>, <eranian@...gle.com>
Subject: Re: [PATCH v16 27/34] fs/resctrl: Introduce mbm_assign_on_mkdir to
 enable assignments on mkdir

Hi Babu,

On 7/25/25 11:29 AM, Babu Moger wrote:
> The "mbm_event" counter assignment mode allows users to assign a hardware
> counter to an RMID, event pair and monitor the bandwidth as long as it is
> assigned.

Above implies this addition is in support of "mbm_event" mode while the
implementation applies to any and all assignable counter modes, including
the "default" and for example the upcoming "soft-ABMC". It is clear to me
how this is used and interpreted when "mbm_event" mode is enabled, but not
for the others (more below).

> 
> Introduce a user-configurable option that determines if a counter will
> automatically be assigned to an RMID, event pair when its associated
> monitor group is created via mkdir.
> 
> Suggested-by: Peter Newman <peternewman@...gle.com>
> Signed-off-by: Babu Moger <babu.moger@....com>
> ---

...

> ---
>  Documentation/filesystems/resctrl.rst | 16 ++++++++++
>  fs/resctrl/monitor.c                  |  2 ++
>  fs/resctrl/rdtgroup.c                 | 43 +++++++++++++++++++++++++++
>  include/linux/resctrl.h               |  3 ++
>  4 files changed, 64 insertions(+)
> 
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 37dbad4d50f7..165e0d315af7 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -354,6 +354,22 @@ with the following files:
>  	  # cat /sys/fs/resctrl/info/L3_MON/event_configs/mbm_total_bytes/event_filter
>  	   local_reads,local_non_temporal_writes
>  
> +"mbm_assign_on_mkdir":

Needs a "Exists when "mbm_event" counter assignment mode is supported."?
Also needs clarification on on behavior when "mbm_event" is enabled vs. disabled.

> +	Determines if a counter will automatically be assigned to an RMID, event pair

"will automatically be" -> "is automatically"
"RMID, event" -> "RMID, MBM event"

> +	when its associated monitor group is created via mkdir. It is enabled by default
> +	on boot and users can disable by writing to the interface.

"users can disable" -> "users can disable this capability" or "can be disabled"?

This implementation enables user to read/write this file/property when "mbm_event" mode is
disabled. Considering this explanation I do not think it is clear how this file reflects
system behavior when in "default" mode. There is no difference between mbm_assign_on_mkdir
enabled/disabled when in "default" mode, no? 
Should interactions with "mbm_assign_on_mkdir" be restricted to when
"mbm_event" mode is enabled? If so, the next question would likely be whether value
should change during "mbm_event" enable->disable or "disable->enable". Above states
clearly that it is enabled on boot and it may be reasonable to have it keep (but not always
expose) user's setting when switching between modes.

Restricting it to "mbm_event" mode now gives us some flexibility when soft-ABMC follows
on if/how it can/should support this. What do you think?

> +
> +	"0":
> +		Auto assignment is disabled.
> +	"1":
> +		Auto assignment is enabled.
> +
> +	Example::
> +
> +	  # echo 0 > /sys/fs/resctrl/info/L3_MON/mbm_assign_on_mkdir
> +	  # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_on_mkdir
> +	  0
> +
>  "max_threshold_occupancy":
>  		Read/write file provides the largest value (in
>  		bytes) at which a previously used LLC_occupancy
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index 8efbeb910f77..6205bbfe08fb 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -1077,6 +1077,8 @@ int resctrl_mon_resource_init(void)
>  		resctrl_file_fflags_init("available_mbm_cntrs",
>  					 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>  		resctrl_file_fflags_init("event_filter", RFTYPE_ASSIGN_CONFIG);
> +		resctrl_file_fflags_init("mbm_assign_on_mkdir", RFTYPE_MON_INFO |
> +					 RFTYPE_RES_CACHE);
>  	}
>  
>  	return 0;
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index c3d6540c3280..bf04235d2603 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c

Please move resctrl_mbm_assign_on_mkdir_show() and resctrl_mbm_assign_on_mkdir_write() to monitor.c

Reinette