Convert update_task_closid_rmid from smp_call_function_single
to smp_call_func_single_fail, which will fail in case
the target CPU is tagged as block interference CPU.
Signed-off-by: Marcelo Tosatti <[email protected]>
Index: linux-isolation/arch/x86/kernel/cpu/resctrl/rdtgroup.c
===================================================================
--- linux-isolation.orig/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ linux-isolation/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -26,6 +26,7 @@
#include <linux/slab.h>
#include <linux/task_work.h>
#include <linux/user_namespace.h>
+#include <linux/sched/isolation.h>
#include <uapi/linux/magic.h>
@@ -551,12 +552,20 @@ static void _update_task_closid_rmid(voi
resctrl_sched_in(task);
}
-static void update_task_closid_rmid(struct task_struct *t)
+static int update_task_closid_rmid(struct task_struct *t)
{
- if (IS_ENABLED(CONFIG_SMP) && task_curr(t))
- smp_call_function_single(task_cpu(t), _update_task_closid_rmid, t, 1);
- else
+ int idx, ret = 0;
+
+ if (IS_ENABLED(CONFIG_SMP) && task_curr(t)) {
+ idx = block_interf_srcu_read_lock();
+ ret = smp_call_function_single_fail(task_cpu(t),
+ _update_task_closid_rmid,
+ t, 1);
+ block_interf_srcu_read_unlock(idx);
+ } else
_update_task_closid_rmid(t);
+
+ return ret;
}
static int __rdtgroup_move_task(struct task_struct *tsk,
@@ -604,9 +613,7 @@ static int __rdtgroup_move_task(struct t
* group go into effect. If the task is not current, the MSR will be
* updated when the task is scheduled in.
*/
- update_task_closid_rmid(tsk);
-
- return 0;
+ return update_task_closid_rmid(tsk);
}
static bool is_closid_match(struct task_struct *t, struct rdtgroup *r)
On Tue, Feb 06 2024 at 15:49, Marcelo Tosatti wrote:
> Convert update_task_closid_rmid from smp_call_function_single
> to smp_call_func_single_fail, which will fail in case
> the target CPU is tagged as block interference CPU.
You fail again to provide a rationale for this change.
What's worse is that you fail to explain why you think that creating
inconistent state is a valid approach.
> Signed-off-by: Marcelo Tosatti <[email protected]>
>
> Index: linux-isolation/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> ===================================================================
> --- linux-isolation.orig/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ linux-isolation/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -26,6 +26,7 @@
> #include <linux/slab.h>
> #include <linux/task_work.h>
> #include <linux/user_namespace.h>
> +#include <linux/sched/isolation.h>
>
> #include <uapi/linux/magic.h>
>
> @@ -551,12 +552,20 @@ static void _update_task_closid_rmid(voi
> resctrl_sched_in(task);
> }
>
> -static void update_task_closid_rmid(struct task_struct *t)
> +static int update_task_closid_rmid(struct task_struct *t)
> {
> - if (IS_ENABLED(CONFIG_SMP) && task_curr(t))
> - smp_call_function_single(task_cpu(t), _update_task_closid_rmid, t, 1);
> - else
> + int idx, ret = 0;
> +
> + if (IS_ENABLED(CONFIG_SMP) && task_curr(t)) {
> + idx = block_interf_srcu_read_lock();
> + ret = smp_call_function_single_fail(task_cpu(t),
> + _update_task_closid_rmid,
> + t, 1);
> + block_interf_srcu_read_unlock(idx);
> + } else
> _update_task_closid_rmid(t);
> +
> + return ret;
This is invoked _after_ the change has been committed to the in-memory
state so how is failing here correct?
Thanks,
tglx
On Mon, Feb 12, 2024 at 04:19:19PM +0100, Thomas Gleixner wrote:
> On Tue, Feb 06 2024 at 15:49, Marcelo Tosatti wrote:
> > Convert update_task_closid_rmid from smp_call_function_single
> > to smp_call_func_single_fail, which will fail in case
> > the target CPU is tagged as block interference CPU.
>
> You fail again to provide a rationale for this change.
>
> What's worse is that you fail to explain why you think that creating
> inconistent state is a valid approach.
Well, the patch is broken and needs fixing.