Re: [RFC PATCH 16/19] fs/resctrl: Implement rdtgroup_plza_write() to configure PLZA in a group

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

Hi Babu,

On 1/21/26 1:12 PM, Babu Moger wrote:
> +static ssize_t rdtgroup_plza_write(struct kernfs_open_file *of, char *buf,
> +				   size_t nbytes, loff_t off)
> +{
> +	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);

Hardcoding PLZA configuration to the L3 resource is unexpected, especially since
PLZA's impact and configuration on MBA is mentioned a couple of times in this
series and discussions that followed. There also does not seem to be any
"per resource" PLZA capability but instead when system supports PLZA 
RDT_RESOURCE_L2, RDT_RESOURCE_L3, and RDT_RESOURCE_MBA are automatically (if
resources are present) set to support it.

From what I understand PLZA enables user space to configure CLOSID and RMID
used in CPL=0 independent from resource. That is, when a user configures
PLZA with this interface all allocation information for all resources in
resource group's schemata applies.

Since this implementation makes "plza" a per-resource property it makes possible
scenarios where some resources support plza while others do not. From what I
can tell this is not reflected by the schemata file associated with a
"plza" resource group that continues to enable user space to change
allocations of all resources, whether they support plza or not.

Why was PLZA determined to be a per-resource property? It instead seems to
have larger scope? The cycle introduced in patch #9 where the arch sets
a per-'resctrl fs' resource property and then forces resctrl fs to query
the arch for its own property seems unnecessary. Could this support just
be a global property that resctrl fs can query from the arch?

> +	struct rdtgroup *rdtgrp, *prgrp;
> +	int cpu, ret = 0;
> +	bool enable;
> +
> +	ret = kstrtobool(buf, &enable);
> +	if (ret)
> +		return ret;
> +
> +	rdtgrp = rdtgroup_kn_lock_live(of->kn);
> +	if (!rdtgrp) {
> +		rdtgroup_kn_unlock(of->kn);
> +		return -ENOENT;
> +	}
> +
> +	rdt_last_cmd_clear();
> +
> +	if (!r->plza_capable) {
> +		rdt_last_cmd_puts("PLZA is not supported in the system\n");
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	if (rdtgrp == &rdtgroup_default) {
> +		rdt_last_cmd_puts("Cannot set PLZA on a default group\n");
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED) {
> +		rdt_last_cmd_puts("Resource group is pseudo-locked\n");
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	if (!list_empty(&rdtgrp->mon.crdtgrp_list)) {
> +		rdt_last_cmd_puts("Cannot change CTRL_MON group with sub monitor groups\n");
> +		ret = -EINVAL;
> +		goto unlock;
> +	}

From what I can tell it is still possible to add monitor groups after a 
CTRL_MON group is designated "plza".

If repurposing a CTRL_MON group to operate with different constraints we should
take care how user can still continue to interact with existing files/directories
as a group transitions between plza and non-plza. One option could be to hide files
as needed to prevent user from interacting with them, another option needs to add
extra checks on all the paths that interact with these files and directories.

Reinette