[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1395b692-cc65-4d84-a68e-f9b20c37f74a@intel.com>
Date: Thu, 22 May 2025 21:48:44 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Babu Moger <babu.moger@....com>, <corbet@....net>, <tony.luck@...el.com>,
<tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
<dave.hansen@...ux.intel.com>
CC: <james.morse@....com>, <dave.martin@....com>, <fenghuay@...dia.com>,
<x86@...nel.org>, <hpa@...or.com>, <paulmck@...nel.org>,
<akpm@...ux-foundation.org>, <thuth@...hat.com>, <rostedt@...dmis.org>,
<ardb@...nel.org>, <gregkh@...uxfoundation.org>,
<daniel.sneddon@...ux.intel.com>, <jpoimboe@...nel.org>,
<alexandre.chartre@...cle.com>, <pawan.kumar.gupta@...ux.intel.com>,
<thomas.lendacky@....com>, <perry.yuan@....com>, <seanjc@...gle.com>,
<kai.huang@...el.com>, <xiaoyao.li@...el.com>, <kan.liang@...ux.intel.com>,
<xin3.li@...el.com>, <ebiggers@...gle.com>, <xin@...or.com>,
<sohil.mehta@...el.com>, <andrew.cooper3@...rix.com>,
<mario.limonciello@....com>, <linux-doc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <peternewman@...gle.com>,
<maciej.wieczor-retman@...el.com>, <eranian@...gle.com>,
<Xiaojian.Du@....com>, <gautham.shenoy@....com>
Subject: Re: [PATCH v13 21/27] x86/resctrl: Introduce mbm_assign_on_mkdir to
configure assignments
Hi Babu,
On 5/15/25 3:52 PM, Babu Moger wrote:
> The mbm_cntr_assign mode provides an option to the user to assign a
> counter to an RMID, event pair and monitor the bandwidth as long as
> the counter is assigned.
>
> Introduce a configuration option to automatically assign counter IDs
"assign counter IDs" -> "assign counter IDs to <what?>"
> when a resctrl group is created, provided the counters are available.
> By default, this option is enabled at boot.
>
> Suggested-by: Peter Newman <peternewman@...gle.com>
> Signed-off-by: Babu Moger <babu.moger@....com>
> ---
> v13: Added Suggested-by tag.
> Resolved conflicts caused by the recent FS/ARCH code restructure.
> The rdtgroup.c/monitor.c file has now been split between the FS and ARCH directories.
>
> v12: New patch. Added after the discussion on the list.
> https://lore.kernel.org/lkml/CALPaoCh8siZKjL_3yvOYGL4cF_n_38KpUFgHVGbQ86nD+Q2_SA@mail.gmail.com/
> ---
> Documentation/filesystems/resctrl.rst | 10 ++++++
> fs/resctrl/monitor.c | 2 ++
> fs/resctrl/rdtgroup.c | 44 +++++++++++++++++++++++++--
> include/linux/resctrl.h | 2 ++
> 4 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 9923276826db..356f1f918a86 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -348,6 +348,16 @@ with the following files:
> # cat /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_total_bytes/event_filter
> local_reads, local_non_temporal_writes
>
> +"mbm_assign_on_mkdir":
> + Automatically assign the monitoring counters on resctrl group creation
assign the monitoring counters to what?
> + if the counters are available. It is enabled by default on boot and users
> + can disable by writing to the interface.
> + ::
> +
> + # echo 0 > /sys/fs/resctrl/info/L3_MON/mbm_assign_on_mkdir
> + # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_on_mkdir
> + 0
Please be explicit in docs what possible values are and what they mean.
> +
> "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 1f72249a5c93..5f6c4b662f3b 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -933,6 +933,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 8c498b41be5d..0093b323d858 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -2035,8 +2035,8 @@ static ssize_t event_filter_write(struct kernfs_open_file *of, char *buf,
> }
>
> ret = resctrl_process_configs(buf, &evt_cfg);
> - if (!ret && mevt->evt_val != evt_cfg) {
> - mevt->evt_val = evt_cfg;
> + if (!ret && mevt->evt_cfg != evt_cfg) {
> + mevt->evt_cfg = evt_cfg;
> resctrl_update_assign(r, mevt->evtid, evt_cfg);
> }
>
Needs to be squashed.
> @@ -2047,6 +2047,39 @@ static ssize_t event_filter_write(struct kernfs_open_file *of, char *buf,
> return ret ?: nbytes;
> }
>
> +static int resctrl_mbm_assign_on_mkdir_show(struct kernfs_open_file *of,
> + struct seq_file *s, void *v)
> +{
> + struct rdt_resource *r = rdt_kn_parent_priv(of->kn);
> +
> + seq_printf(s, "%u\n", r->mon.mbm_assign_on_mkdir);
> +
> + return 0;
> +}
> +
> +static ssize_t resctrl_mbm_assign_on_mkdir_write(struct kernfs_open_file *of,
> + char *buf, size_t nbytes, loff_t off)
> +{
> + struct rdt_resource *r = rdt_kn_parent_priv(of->kn);
> + bool value;
> + int ret;
> +
> + ret = kstrtobool(buf, &value);
> + if (ret)
> + return ret;
> +
> + cpus_read_lock();
not traversing the domain list so hotplug lock not needed.
> + mutex_lock(&rdtgroup_mutex);
rdtgroup_mutex seems only needed because the message buffer is cleared below, and this is why it
is not required in the show()?
> + rdt_last_cmd_clear();
> +
> + r->mon.mbm_assign_on_mkdir = value;
> +
> + mutex_unlock(&rdtgroup_mutex);
> + cpus_read_unlock();
> +
> + return ret ?: nbytes;
> +}
> +
> /* rdtgroup information files for one cache resource. */
> static struct rftype res_common_files[] = {
> {
> @@ -2056,6 +2089,13 @@ static struct rftype res_common_files[] = {
> .seq_show = rdt_last_cmd_status_show,
> .fflags = RFTYPE_TOP_INFO,
> },
> + {
> + .name = "mbm_assign_on_mkdir",
> + .mode = 0644,
> + .kf_ops = &rdtgroup_kf_single_ops,
> + .seq_show = resctrl_mbm_assign_on_mkdir_show,
> + .write = resctrl_mbm_assign_on_mkdir_write,
> + },
> {
> .name = "num_closids",
> .mode = 0444,
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index cd24d1577e0a..d6435abdde7b 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -278,6 +278,7 @@ enum resctrl_schema_fmt {
> * monitoring events can be configured.
> * @num_mbm_cntrs: Number of assignable monitoring counters
> * @mbm_cntr_assignable:Is system capable of supporting monitor assignment?
> + * @mbm_assign_on_mkdir:Auto enable monitor assignment on mkdir?
How is "monitor assignment" different from "counter assignment"?
> * @evt_list: List of monitoring events
> */
> struct resctrl_mon {
> @@ -285,6 +286,7 @@ struct resctrl_mon {
> unsigned int mbm_cfg_mask;
> int num_mbm_cntrs;
> bool mbm_cntr_assignable;
> + bool mbm_assign_on_mkdir;
> struct list_head evt_list;
> };
>
Reinette
Powered by blists - more mailing lists