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: <2c3a8282-e876-4cdb-8355-0fd78eb6c0b4@intel.com>
Date: Wed, 17 Sep 2025 22:43:14 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Babu Moger <babu.moger@....com>, <corbet@....net>, <tony.luck@...el.com>,
	<Dave.Martin@....com>, <james.morse@....com>, <tglx@...utronix.de>,
	<mingo@...hat.com>, <bp@...en8.de>, <dave.hansen@...ux.intel.com>
CC: <x86@...nel.org>, <hpa@...or.com>, <kas@...nel.org>,
	<rick.p.edgecombe@...el.com>, <akpm@...ux-foundation.org>,
	<paulmck@...nel.org>, <pmladek@...e.com>,
	<pawan.kumar.gupta@...ux.intel.com>, <rostedt@...dmis.org>,
	<kees@...nel.org>, <arnd@...db.de>, <fvdl@...gle.com>, <seanjc@...gle.com>,
	<thomas.lendacky@....com>, <manali.shukla@....com>, <perry.yuan@....com>,
	<sohil.mehta@...el.com>, <xin@...or.com>, <peterz@...radead.org>,
	<mario.limonciello@....com>, <gautham.shenoy@....com>, <nikunj@....com>,
	<dapeng1.mi@...ux.intel.com>, <ak@...ux.intel.com>,
	<chang.seok.bae@...el.com>, <ebiggers@...gle.com>,
	<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-coco@...ts.linux.dev>, <kvm@...r.kernel.org>
Subject: Re: [PATCH v9 07/10] fs/resctrl: Introduce interface to display
 io_alloc CBMs

Hi Babu,

On 9/2/25 3:41 PM, Babu Moger wrote:
> The io_alloc feature in resctrl enables system software to configure
> the portion of the cache allocated for I/O traffic.

(repetitive)

> 
> Add "io_alloc_cbm" resctrl file to display the Capacity Bit Masks (CBMs)
> that represent the portion of each cache instance allocated for I/O
> traffic.
> 
> The CBM interface file io_alloc_cbm resides in the info directory (e.g.,
> /sys/fs/resctrl/info/L3/). Since the resource name is part of the path, it
> is not necessary to display the resource name as done in the schemata file.
> Pass the resource name to show_doms() and print it only if the name is
> valid. For io_alloc, pass NULL pointer to suppress printing the resource
> name.

Related to changelog feedback received during ABMC series I think the portion
that describes the code  (from "Pass the resource name ..." to "printing the
resource name"), is unnecessary since it can be seen by looking at the patch.

> 
> When CDP is enabled, io_alloc routes traffic using the highest CLOSID
> associated with the L3CODE resource. To ensure consistent cache allocation
> behavior, the L3CODE and L3DATA resources are kept in sync. So, the

I do not think the "To ensure consistent cache allocation behavior" is accurate.
This is just to avoid the possible user space confusion if supporting the feature
with L3CODE used for I/O allocation and L3DATA becomes unusable, no?

Also, this also needs to be in imperative tone.

> Capacity Bit Masks (CBMs) accessed through either L3CODE or L3DATA will
> reflect identical values.

Attempt to put it together, please feel free to improve:

	Introduce the "io_alloc_cbm" resctrl file to display the Capacity Bit
	Masks (CBMs) that represent the portions of each cache instance allocated
	for I/O traffic on a cache resource that supports the "io_alloc" feature.         

	io_alloc_cbm resides in the info directory of a cache resource, for example,
	/sys/fs/resctrl/info/L3/. Since the resource name is part of the path, it
	is not necessary to display the resource name as done in the schemata file.

	When CDP is enabled, io_alloc routes traffic using the highest CLOSID
	associated with the L3CODE resource and that CLOSID becomes unusable for
	the L3DATA resource. The highest CLOSID of L3CODE and L3DATA resources will
	be kept	in sync	to ensure consistent user interface. In preparation for this,
	access the CBMs for I/O traffic through highest CLOSID of either L3CODE or
	L3DATA resource.

> 
> Signed-off-by: Babu Moger <babu.moger@....com>
> ---

..

> @@ -807,3 +809,40 @@ ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
>  
>  	return ret ?: nbytes;
>  }
> +
> +int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq, void *v)
> +{
> +	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
> +	struct rdt_resource *r = s->res;
> +	int ret = 0;
> +
> +	cpus_read_lock();
> +	mutex_lock(&rdtgroup_mutex);
> +
> +	rdt_last_cmd_clear();
> +
> +	if (!r->cache.io_alloc_capable) {
> +		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
> +		ret = -ENODEV;
> +		goto out_unlock;
> +	}
> +
> +	if (!resctrl_arch_get_io_alloc_enabled(r)) {
> +		rdt_last_cmd_printf("io_alloc is not enabled on %s\n", s->name);
> +		ret = -EINVAL;

The return code when io_alloc is not enabled is different between reading from (EINVAL) and
writing to (ENODEV) io_alloc_cbm. Please be consistent.

> +		goto out_unlock;
> +	}
> +
> +	/*
> +	 * When CDP is enabled, resctrl_io_alloc_init_cbm() sets the same CBM for
> +	 * both L3CODE and L3DATA of the highest CLOSID. As a result, the io_alloc

Not just during initialization, they are kept in sync during runtime also (when
user writes to io_alloc_cbm). First sentence can perhaps just be
	"When CDP is enabled the CBMs of the highest CLOSID of L3CODE
	 and L3DATA are kept in sync. As a result, ..."

Reinette

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ