Re: [PATCH v2 1/5] fs/resctrl: Move functions to avoid forward references in subsequent fixes

[Babu Moger <babu.moger@amd.com>]

Hi Tony,

On 5/15/26 14:39, Tony Luck wrote:
> No functional change. Just pull some functions before rdt_get_tree().

Would it be better to list functions instead of "some functions" ?

Thanks
Babu

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>   fs/resctrl/rdtgroup.c | 376 +++++++++++++++++++++---------------------
>   1 file changed, 188 insertions(+), 188 deletions(-)
> 
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 5dfdaa6f9d8f..a6376a3fc4c3 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -2782,6 +2782,194 @@ static void schemata_list_destroy(void)
>   	}
>   }
>   
> +/*
> + * Move tasks from one to the other group. If @from is NULL, then all tasks
> + * in the systems are moved unconditionally (used for teardown).
> + *
> + * If @mask is not NULL the cpus on which moved tasks are running are set
> + * in that mask so the update smp function call is restricted to affected
> + * cpus.
> + */
> +static void rdt_move_group_tasks(struct rdtgroup *from, struct rdtgroup *to,
> +				 struct cpumask *mask)
> +{
> +	struct task_struct *p, *t;
> +
> +	read_lock(&tasklist_lock);
> +	for_each_process_thread(p, t) {
> +		if (!from || is_closid_match(t, from) ||
> +		    is_rmid_match(t, from)) {
> +			resctrl_arch_set_closid_rmid(t, to->closid,
> +						     to->mon.rmid);
> +
> +			/*
> +			 * Order the closid/rmid stores above before the loads
> +			 * in task_curr(). This pairs with the full barrier
> +			 * between the rq->curr update and
> +			 * resctrl_arch_sched_in() during context switch.
> +			 */
> +			smp_mb();
> +
> +			/*
> +			 * If the task is on a CPU, set the CPU in the mask.
> +			 * The detection is inaccurate as tasks might move or
> +			 * schedule before the smp function call takes place.
> +			 * In such a case the function call is pointless, but
> +			 * there is no other side effect.
> +			 */
> +			if (IS_ENABLED(CONFIG_SMP) && mask && task_curr(t))
> +				cpumask_set_cpu(task_cpu(t), mask);
> +		}
> +	}
> +	read_unlock(&tasklist_lock);
> +}
> +
> +static void free_all_child_rdtgrp(struct rdtgroup *rdtgrp)
> +{
> +	struct rdtgroup *sentry, *stmp;
> +	struct list_head *head;
> +
> +	head = &rdtgrp->mon.crdtgrp_list;
> +	list_for_each_entry_safe(sentry, stmp, head, mon.crdtgrp_list) {
> +		rdtgroup_unassign_cntrs(sentry);
> +		free_rmid(sentry->closid, sentry->mon.rmid);
> +		list_del(&sentry->mon.crdtgrp_list);
> +
> +		if (atomic_read(&sentry->waitcount) != 0)
> +			sentry->flags = RDT_DELETED;
> +		else
> +			rdtgroup_remove(sentry);
> +	}
> +}
> +
> +/*
> + * Forcibly remove all of subdirectories under root.
> + */
> +static void rmdir_all_sub(void)
> +{
> +	struct rdtgroup *rdtgrp, *tmp;
> +
> +	/* Move all tasks to the default resource group */
> +	rdt_move_group_tasks(NULL, &rdtgroup_default, NULL);
> +
> +	list_for_each_entry_safe(rdtgrp, tmp, &rdt_all_groups, rdtgroup_list) {
> +		/* Free any child rmids */
> +		free_all_child_rdtgrp(rdtgrp);
> +
> +		/* Remove each rdtgroup other than root */
> +		if (rdtgrp == &rdtgroup_default)
> +			continue;
> +
> +		if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP ||
> +		    rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED)
> +			rdtgroup_pseudo_lock_remove(rdtgrp);
> +
> +		/*
> +		 * Give any CPUs back to the default group. We cannot copy
> +		 * cpu_online_mask because a CPU might have executed the
> +		 * offline callback already, but is still marked online.
> +		 */
> +		cpumask_or(&rdtgroup_default.cpu_mask,
> +			   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
> +
> +		rdtgroup_unassign_cntrs(rdtgrp);
> +
> +		free_rmid(rdtgrp->closid, rdtgrp->mon.rmid);
> +
> +		kernfs_remove(rdtgrp->kn);
> +		list_del(&rdtgrp->rdtgroup_list);
> +
> +		if (atomic_read(&rdtgrp->waitcount) != 0)
> +			rdtgrp->flags = RDT_DELETED;
> +		else
> +			rdtgroup_remove(rdtgrp);
> +	}
> +	/* Notify online CPUs to update per cpu storage and PQR_ASSOC MSR */
> +	update_closid_rmid(cpu_online_mask, &rdtgroup_default);
> +
> +	kernfs_remove(kn_info);
> +	kernfs_remove(kn_mongrp);
> +	kernfs_remove(kn_mondata);
> +}
> +
> +/**
> + * mon_get_kn_priv() - Get the mon_data priv data for this event.
> + *
> + * The same values are used across the mon_data directories of all control and
> + * monitor groups for the same event in the same domain. Keep a list of
> + * allocated structures and re-use an existing one with the same values for
> + * @rid, @domid, etc.
> + *
> + * @rid:    The resource id for the event file being created.
> + * @domid:  The domain id for the event file being created.
> + * @mevt:   The type of event file being created.
> + * @do_sum: Whether SNC summing monitors are being created. Only set
> + *	    when @rid == RDT_RESOURCE_L3.
> + *
> + * Return: Pointer to mon_data private data of the event, NULL on failure.
> + */
> +static struct mon_data *mon_get_kn_priv(enum resctrl_res_level rid, int domid,
> +					struct mon_evt *mevt,
> +					bool do_sum)
> +{
> +	struct mon_data *priv;
> +
> +	lockdep_assert_held(&rdtgroup_mutex);
> +
> +	list_for_each_entry(priv, &mon_data_kn_priv_list, list) {
> +		if (priv->rid == rid && priv->domid == domid &&
> +		    priv->sum == do_sum && priv->evt == mevt)
> +			return priv;
> +	}
> +
> +	priv = kzalloc_obj(*priv);
> +	if (!priv)
> +		return NULL;
> +
> +	priv->rid = rid;
> +	priv->domid = domid;
> +	priv->sum = do_sum;
> +	priv->evt = mevt;
> +	list_add_tail(&priv->list, &mon_data_kn_priv_list);
> +
> +	return priv;
> +}
> +
> +/**
> + * mon_put_kn_priv() - Free all allocated mon_data structures.
> + *
> + * Called when resctrl file system is unmounted.
> + */
> +static void mon_put_kn_priv(void)
> +{
> +	struct mon_data *priv, *tmp;
> +
> +	lockdep_assert_held(&rdtgroup_mutex);
> +
> +	list_for_each_entry_safe(priv, tmp, &mon_data_kn_priv_list, list) {
> +		list_del(&priv->list);
> +		kfree(priv);
> +	}
> +}
> +
> +static void resctrl_fs_teardown(void)
> +{
> +	lockdep_assert_held(&rdtgroup_mutex);
> +
> +	/* Cleared by rdtgroup_destroy_root() */
> +	if (!rdtgroup_default.kn)
> +		return;
> +
> +	rmdir_all_sub();
> +	rdtgroup_unassign_cntrs(&rdtgroup_default);
> +	mon_put_kn_priv();
> +	rdt_pseudo_lock_release();
> +	rdtgroup_default.mode = RDT_MODE_SHAREABLE;
> +	closid_exit();
> +	schemata_list_destroy();
> +	rdtgroup_destroy_root();
> +}
> +
>   static int rdt_get_tree(struct fs_context *fc)
>   {
>   	struct rdt_fs_context *ctx = rdt_fc2context(fc);
> @@ -2981,194 +3169,6 @@ static int rdt_init_fs_context(struct fs_context *fc)
>   	return 0;
>   }
>   
> -/*
> - * Move tasks from one to the other group. If @from is NULL, then all tasks
> - * in the systems are moved unconditionally (used for teardown).
> - *
> - * If @mask is not NULL the cpus on which moved tasks are running are set
> - * in that mask so the update smp function call is restricted to affected
> - * cpus.
> - */
> -static void rdt_move_group_tasks(struct rdtgroup *from, struct rdtgroup *to,
> -				 struct cpumask *mask)
> -{
> -	struct task_struct *p, *t;
> -
> -	read_lock(&tasklist_lock);
> -	for_each_process_thread(p, t) {
> -		if (!from || is_closid_match(t, from) ||
> -		    is_rmid_match(t, from)) {
> -			resctrl_arch_set_closid_rmid(t, to->closid,
> -						     to->mon.rmid);
> -
> -			/*
> -			 * Order the closid/rmid stores above before the loads
> -			 * in task_curr(). This pairs with the full barrier
> -			 * between the rq->curr update and
> -			 * resctrl_arch_sched_in() during context switch.
> -			 */
> -			smp_mb();
> -
> -			/*
> -			 * If the task is on a CPU, set the CPU in the mask.
> -			 * The detection is inaccurate as tasks might move or
> -			 * schedule before the smp function call takes place.
> -			 * In such a case the function call is pointless, but
> -			 * there is no other side effect.
> -			 */
> -			if (IS_ENABLED(CONFIG_SMP) && mask && task_curr(t))
> -				cpumask_set_cpu(task_cpu(t), mask);
> -		}
> -	}
> -	read_unlock(&tasklist_lock);
> -}
> -
> -static void free_all_child_rdtgrp(struct rdtgroup *rdtgrp)
> -{
> -	struct rdtgroup *sentry, *stmp;
> -	struct list_head *head;
> -
> -	head = &rdtgrp->mon.crdtgrp_list;
> -	list_for_each_entry_safe(sentry, stmp, head, mon.crdtgrp_list) {
> -		rdtgroup_unassign_cntrs(sentry);
> -		free_rmid(sentry->closid, sentry->mon.rmid);
> -		list_del(&sentry->mon.crdtgrp_list);
> -
> -		if (atomic_read(&sentry->waitcount) != 0)
> -			sentry->flags = RDT_DELETED;
> -		else
> -			rdtgroup_remove(sentry);
> -	}
> -}
> -
> -/*
> - * Forcibly remove all of subdirectories under root.
> - */
> -static void rmdir_all_sub(void)
> -{
> -	struct rdtgroup *rdtgrp, *tmp;
> -
> -	/* Move all tasks to the default resource group */
> -	rdt_move_group_tasks(NULL, &rdtgroup_default, NULL);
> -
> -	list_for_each_entry_safe(rdtgrp, tmp, &rdt_all_groups, rdtgroup_list) {
> -		/* Free any child rmids */
> -		free_all_child_rdtgrp(rdtgrp);
> -
> -		/* Remove each rdtgroup other than root */
> -		if (rdtgrp == &rdtgroup_default)
> -			continue;
> -
> -		if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP ||
> -		    rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED)
> -			rdtgroup_pseudo_lock_remove(rdtgrp);
> -
> -		/*
> -		 * Give any CPUs back to the default group. We cannot copy
> -		 * cpu_online_mask because a CPU might have executed the
> -		 * offline callback already, but is still marked online.
> -		 */
> -		cpumask_or(&rdtgroup_default.cpu_mask,
> -			   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
> -
> -		rdtgroup_unassign_cntrs(rdtgrp);
> -
> -		free_rmid(rdtgrp->closid, rdtgrp->mon.rmid);
> -
> -		kernfs_remove(rdtgrp->kn);
> -		list_del(&rdtgrp->rdtgroup_list);
> -
> -		if (atomic_read(&rdtgrp->waitcount) != 0)
> -			rdtgrp->flags = RDT_DELETED;
> -		else
> -			rdtgroup_remove(rdtgrp);
> -	}
> -	/* Notify online CPUs to update per cpu storage and PQR_ASSOC MSR */
> -	update_closid_rmid(cpu_online_mask, &rdtgroup_default);
> -
> -	kernfs_remove(kn_info);
> -	kernfs_remove(kn_mongrp);
> -	kernfs_remove(kn_mondata);
> -}
> -
> -/**
> - * mon_get_kn_priv() - Get the mon_data priv data for this event.
> - *
> - * The same values are used across the mon_data directories of all control and
> - * monitor groups for the same event in the same domain. Keep a list of
> - * allocated structures and re-use an existing one with the same values for
> - * @rid, @domid, etc.
> - *
> - * @rid:    The resource id for the event file being created.
> - * @domid:  The domain id for the event file being created.
> - * @mevt:   The type of event file being created.
> - * @do_sum: Whether SNC summing monitors are being created. Only set
> - *	    when @rid == RDT_RESOURCE_L3.
> - *
> - * Return: Pointer to mon_data private data of the event, NULL on failure.
> - */
> -static struct mon_data *mon_get_kn_priv(enum resctrl_res_level rid, int domid,
> -					struct mon_evt *mevt,
> -					bool do_sum)
> -{
> -	struct mon_data *priv;
> -
> -	lockdep_assert_held(&rdtgroup_mutex);
> -
> -	list_for_each_entry(priv, &mon_data_kn_priv_list, list) {
> -		if (priv->rid == rid && priv->domid == domid &&
> -		    priv->sum == do_sum && priv->evt == mevt)
> -			return priv;
> -	}
> -
> -	priv = kzalloc_obj(*priv);
> -	if (!priv)
> -		return NULL;
> -
> -	priv->rid = rid;
> -	priv->domid = domid;
> -	priv->sum = do_sum;
> -	priv->evt = mevt;
> -	list_add_tail(&priv->list, &mon_data_kn_priv_list);
> -
> -	return priv;
> -}
> -
> -/**
> - * mon_put_kn_priv() - Free all allocated mon_data structures.
> - *
> - * Called when resctrl file system is unmounted.
> - */
> -static void mon_put_kn_priv(void)
> -{
> -	struct mon_data *priv, *tmp;
> -
> -	lockdep_assert_held(&rdtgroup_mutex);
> -
> -	list_for_each_entry_safe(priv, tmp, &mon_data_kn_priv_list, list) {
> -		list_del(&priv->list);
> -		kfree(priv);
> -	}
> -}
> -
> -static void resctrl_fs_teardown(void)
> -{
> -	lockdep_assert_held(&rdtgroup_mutex);
> -
> -	/* Cleared by rdtgroup_destroy_root() */
> -	if (!rdtgroup_default.kn)
> -		return;
> -
> -	rmdir_all_sub();
> -	rdtgroup_unassign_cntrs(&rdtgroup_default);
> -	mon_put_kn_priv();
> -	rdt_pseudo_lock_release();
> -	rdtgroup_default.mode = RDT_MODE_SHAREABLE;
> -	closid_exit();
> -	schemata_list_destroy();
> -	rdtgroup_destroy_root();
> -}
> -
>   static void rdt_kill_sb(struct super_block *sb)
>   {
>   	struct rdt_resource *r;