Re: [PATCH] sched/deadline: Fix priority inheritance with multiple scheduling classes

[Valentin Schneider <valentin.schneider@arm.com>]

On 05/11/20 16:33, Daniel Bristot de Oliveira wrote:
> On 11/5/20 5:12 PM, Juri Lelli wrote:
>> On 05/11/20 15:49, Valentin Schneider wrote:
>>> For my own sake, what affinity problems are you thinking of?
>>>
>>> With proxy exec we have this "funny" dance of shoving the entire blocked-on
>>> chain on a single runqueue to get the right selection out of
>>> pick_next_task(), and that needs to deal with affinity (i.e. move the task
>>> back to a sensible rq once it becomes runnable).
>>>
>>> With the current PI, the waiting tasks are blocked and enqueued in the
>>> pi_waiters tree, so as I see it affinity shouldn't matter; what am I
>>> missing / not seeing? Is that related to bandwidth handling?
>>
>> Think we might break admission control checks if donor and bosted are,
>> for example, on different exclusive sets of CPUs. Guess that is a
>> problem with proxy as well, though.

Right, that gives you different rd's...

>> As said in the comment above, this
>> is unfortunately not much more than a band-aid. Hoping we can buy us
>> some time and fix it properly with proxy.
>
> I agree with Juri that the current approach is known to be broken,
> and that the proxy execution seems to be the mechanisms to go to
> try to address these problems. However, this will take some time.
>
> Meanwhile, this patch that Juri proposes fixes problem
> in the current mechanism - using the same approach (and breaking
> in a known way :D).
>
> A proper way to handle the priority inversion with a disjoint
> set of CPUs is something that will also be an issue with proxy
> execution. But that is an even more complex topic :-(.
>
> So, IMHO, Juri's patch works well to avoid a crash,
> making the system to behave as we expected (even if
> we know that we cannot expect too much).
>

Aye, no disagreement here! I was mainly asking out of "personal interest",
given I too have an eye on proxy exec - and would actually like to get back
to it this month, if my inbox agrees.

>>> With this change, do we still need sched_dl_entity.dl_boosted? AIUI this
>>> could become
>>>
>>>   bool is_dl_boosted(struct sched_dl_entity *dl_se)
>>>   {
>>>           return pi_of(dl_se) != dl_se;
>>>   }
>>
>> Makes sense to me. I'll add this change as a separate patch if the rest
>> makes sense to people as well. :-)
>
> +1

FWIW nothing strikes me as too crazy, so with the above:

Reviewed-by: Valentin Schneider <valentin.schneider@arm.com>

>
> -- Daniel
>
>>
>> Thanks for the quick review!
>>
>> Best,
>> Juri
>>