Re: [PATCH mpam mpam/snapshot+extras/v6.18-rc1 v2 3/3] fs/resctrl: Migrate IOMMU groups when removing resource groups

[Lee Trager]

Hi Zeng,

Thanks for the series, I found it while backporting MPAM support to 6.18.

On 4/13/26 8:26 PM, Zeng Heng wrote:
> When deleting a control group, monitor group, or unmounting the resctrl
> filesystem, migrate all associated IOMMU groups to the appropriate
> destination:
>    * Control group deletion: move IOMMU groups to the default group
>    * Monitor group deletion: move IOMMU groups to the parent control group
>    * Filesystem unmount: move all IOMMU groups to the default group
>
> Without this migration, IOMMU groups remain bound to stale PARTID/PMG
> values of the destroyed group, causing them to "disappear" from the
> resctrl interface.
>
> Add rdt_move_group_iommus() to handle this migration, mirroring the
> existing rdt_move_group_tasks() pattern for task migration.
>
> When deleting a control group or unmounting the resctrl file system, it
> is necessary to move its all iommu_groups back to the default group. When
> removing a monitor group, need to move its iommu_groups back to the parent
> control group.
>
> Otherwise, these iommu_groups remain bound to the old PARTID and PMG, and
> they will appear to "disappear" from the resctrl fs.
>
> Signed-off-by: Zeng Heng <zengheng4@xxxxxxxxxx>
> ---
>   fs/resctrl/rdtgroup.c | 47 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index e92b5dcb6f2e..351e430bde1a 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -3292,6 +3292,44 @@ static void rdt_move_group_tasks(struct rdtgroup *from, struct rdtgroup *to,
>   	read_unlock(&tasklist_lock);
>   }
>   
> +static int rdt_move_group_iommus(struct rdtgroup *from, struct rdtgroup *to)
> +{
> +	struct kset *iommu_groups;
> +	struct iommu_group *group;
> +	int err = 0, iommu_group_id;
> +	struct kobject *group_kobj = NULL;
> +
> +	if (!IS_ENABLED(CONFIG_RESCTRL_IOMMU))
> +		return 0;
> +
> +	if (from == to)
> +		return 0;
> +
> +	iommu_groups = iommu_get_group_kset();
> +
> +	while ((group_kobj = kset_get_next_obj(iommu_groups, group_kobj))) {
> +		/* iommu_group_get_from_kobj() wants to drop a reference */
> +		kobject_get(group_kobj);
> +
> +		group = iommu_group_get_from_kobj(group_kobj);
> +		if (!group)
> +			continue;
> +
> +		if (!from || iommu_matches_rdtgroup(group, from)) {
> +			err = kstrtoint(group_kobj->name, 0, &iommu_group_id);
> +			if (err)
> +				break;
> +
> +			err = rdtgroup_move_iommu(iommu_group_id, to);
> +			if (err)
> +				break;
> +		}
> +	}
> +
> +	kset_put(iommu_groups);
> +	return err;
> +}

iommu_group_get_from_kobj() returns holding a reference on 
group->devices_kobj, the one iommu_group_put() releases. The 
kobject_get() above only compensates for the kobject_put(&group->kobj) 
done internally by iommu_group_get_from_kobj(). Nothing in this loop 
ever calls iommu_group_put(), so this leaks one iommu_group reference  
per group in the kset on every call, e.g on rmdir and every unmount. 
Since device_kobj pins the group's koject, the leaked groups can never 
be freed.

The same pattern exists in show_rdt_iommu() from "fs/resctrl: Add 
support for assigning iommu_groups to resctrl groups", where it leaks 
one reference per group on every read of a tasks file.

There is a second, smaller leak on these break paths: 
kset_get_next_obj() only drops its reference on the current kobject when 
it is passed back as @prev on the next iteration, so breaking out of the 
loop also leaks the group_kobj reference taken by the iterator. I am 
carrying the fix below:

if (!from || iommu_matches_rdtgroup(group, from)) {
	err = kstrtoint(group_kobj->name, 0, &iommu_group_id);
	if (!err)
		err = rdtgroup_move_iommu(iommu_group_id, to);
}

iommu_group_put(group);
if (err) {
	kobject_put(group_kobj);
	break;
}

Thanks,

Lee