Re: resume_oob_task & not actually resuming

[Philippe Gerum <rpm@xenomai.org>]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 30.06.21 19:52, Philippe Gerum wrote:
>> 
>> Philippe Gerum via Xenomai <xenomai@xenomai.org> writes:
>> 
>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>>
>>>> Hi Philippe,
>>>>
>>>> need you guidance here to fix the "thread ... switched to non-rt CPU,
>>>> aborted" issue:
>>>>
>>>> For some reason, I-pipe is fine and kicks the migrating non-rt task
>>>> again when ipipe_migration_hook() does not resume the target thread due
>>>> to failing cobalt_affinity_ok() check. Over dovetail, this is not
>>>> working, and the thread is stuck in nirvana, i.e. suspended as hardened
>>>> from Linux POV but not resumed on the Xenomai side. Looking at how
>>>> finalize_oob_transition() is called in the dovetail kernel, it does not
>>>> seem like it is prepared for not being in oob after that call
>>>> (finish_task_switch is not called - not sure if that makes the difference).
>>>>
>>>> So, either the point of checking and failing the migration in Xenomai is
>>>> wrong for dovetail, or we need some extension of the latter to account
>>>> for that case. What was the intended design?
>>>>
>>>
>>> Dovetail has it right, Cobalt is wrong in this case. Cobalt-wise: we
>>> should always 1) raise a cancellation request upon any issue with
>>> switching to the oob stage on behalf of resume_oob_task(), 2) lift the
>>> XNRELAX suspension bit, 3) detect the pending cancellation in
>>> xnshadow_harden(), forcing the current thread to exit. I did not dive
>>> into the details yet, but I suspect that Cobalt might be lucky with the
>>> I-pipe in skipping xnthread_resume() upon failure (some pending signal,
>>> forcing sigwake maybe?).
>>>
>>> IOW, we should complete the switch to oob in any case, then kick out the
>>> thread with a bad affinity when unwinding from xnshadow_harden().
>> 
>> Of course, this means that all runqueues must be built for all possible
>> CPUs, NOT restricted to the CPUs which are part of the oob-capable
>> set. Cobalt sets up a runqueue for each online CPU in xnsched_init_all()
>> which is more restrictive, but that should still work provided the
>> hot-plug scenario is dismissed.
>
> The hot-plug scenario cannot be dismissed, obviously. It was one of
> motivations to enable partial CPU sets.
>

I did not know that. I-pipe wise, how/when are we turning on the per-cpu
tick diversion for a new CPU when it appears, so that such CPU can run
Xenomai threads?

> I don't think we are on the right way yet. Can't we prevent entering the
> migration procedure in the first place when detecting affinity issues?
> That would be way cleaner than letting a misguided thread pop up on a
> CPU it does not belong or creating data structures (or even more) for
> CPUs Xenomai has no business in.

I'm not sure this would be any cleaner: there are internal callers
fixing up the thread affinity via do_set_cpus_allowed(), which might
then start receiving failure codes returned by some Dovetail hook
denying the move, then what should they do?

The upside of checking the affinity when a thread actually moves back to
oob context is that we can catch any wrongful situation, without
sprinkling the in-band code with checks and hooks, playing whack-a-mole
with issues induced by upstream changes.

Before anything, this hunk should be merged into the Dovetail code
before testing...

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ee6fe5e9c7faaaf..891398dc63aec07 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2384,9 +2384,9 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,
 
 	__do_set_cpus_allowed(p, new_mask, flags);
 
-	return affine_move_task(rq, p, &rf, dest_cpu, flags);
-
 	inband_migration_notify(p, dest_cpu);
+
+	return affine_move_task(rq, p, &rf, dest_cpu, flags);
 out:
 	task_rq_unlock(rq, p, &rf);
 

-- 
Philippe.