Re: [PATCH v1 3/6] x86/resctrl: Disallow mongroup rename on MPAM

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

Hi Peter,

On 3/25/2024 10:27 AM, Peter Newman wrote:
> Moving a monitoring group to a different parent control assumes that the
> monitors will not be impacted. This is not the case on MPAM where the
> PMG is an extension of the PARTID.
> 
> Detect this situation by requiring the change in CLOSID not to affect
> the result of resctrl_arch_rmid_idx_encode(), otherwise return
> -EOPNOTSUPP.
> 

Thanks for catching this. This seems out of place in this series. It sounds
more like an independent fix that should go in separately.

> Signed-off-by: Peter Newman <peternewman@google.com>
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 9b1969e4235a..8d6979dbfd02 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -3879,6 +3879,19 @@ static int rdtgroup_rename(struct kernfs_node *kn,
>  		goto out;
>  	}
>  
> +	/*
> +	 * If changing the CLOSID impacts the RMID, this operation is not
> +	 * supported.
> +	 */
> +	if (resctrl_arch_rmid_idx_encode(rdtgrp->mon.parent->closid,
> +					 rdtgrp->mon.rmid) !=
> +	    resctrl_arch_rmid_idx_encode(new_prdtgrp->closid,
> +					 rdtgrp->mon.rmid)) {
> +		rdt_last_cmd_puts("changing parent control group not supported\n");

Please start message with capital letter.

> +		ret = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
>  	/*
>  	 * If the MON group is monitoring CPUs, the CPUs must be assigned to the
>  	 * current parent CTRL_MON group and therefore cannot be assigned to

Reinette