Hi Reinette, Fenghua,
On 03/12/2020 23:25, Reinette Chatre wrote:
> From: Fenghua Yu <fenghua.yu@xxxxxxxxx>
>
> The code of setting the CPU on which a task is running in a CPU mask is
> moved into a couple of helpers. The new helper task_on_cpu() will be
> reused shortly.
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 6f4ca4bea625..68db7d2dec8f 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -525,6 +525,38 @@ static void rdtgroup_remove(struct rdtgroup *rdtgrp)
> +#ifdef CONFIG_SMP
(using IS_ENABLED(CONFIG_SMP) lets the compiler check all the code in one go, then
dead-code-remove the stuff that will never happen... its also easier on the eye!)
> +/* Get the CPU if the task is on it. */
> +static bool task_on_cpu(struct task_struct *t, int *cpu)
> +{
> + /*
> + * This is safe on x86 w/o barriers as the ordering of writing to
> + * task_cpu() and t->on_cpu is reverse to the reading here. 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 (t->on_cpu) {
kernel/sched/core.c calls out that there can be two tasks on one CPU with this set.
(grep astute)
I think that means this series will falsely match the old task for a CPU while the
scheduler is running, and IPI it unnecessarily.
task_curr() is the helper that knows not to do this.
> + *cpu = task_cpu(t);
> +
> + return true;
> + }
> +
> + return false;
> +}
Thanks,
James
