Re: [PATCH v2] sched/fair: Do not set skip buddy up the sched hierarchy

[Dietmar Eggemann <dietmar.eggemann@arm.com>]

On 12/12/2019 23:19, Josh Don wrote:
> On Mon, Dec 9, 2019 at 1:19 AM Dietmar Eggemann
> <dietmar.eggemann@arm.com> wrote:
>>
>> On 06.12.19 23:13, Josh Don wrote:
>>
>> [...]
>>
>>> On Thu, Dec 5, 2019 at 11:57 PM Vincent Guittot
>>> <vincent.guittot@linaro.org> wrote:
>>>>
>>>> Hi Josh,
>>>>
>>>> On Wed, 4 Dec 2019 at 21:06, Josh Don <joshdon@google.com> wrote:
>>>>>
>>>>> From: Venkatesh Pallipadi <venki@google.com>
>>>>>
>>>>> Setting skip buddy all the way up the hierarchy does not play well
>>>>> with intra-cgroup yield. One typical usecase of yield is when a
>>>>> thread in a cgroup wants to yield CPU to another thread within the
>>>>> same cgroup. For such a case, setting the skip buddy all the way up
>>
>> But with yield_task{_fair}() you have no way to control which other task
>> gets accelerated. The other task in the taskgroup (cgroup) could be even
>> on another CPU.
>>
>> It's not like yield_to_task_fair() which uses next buddy to accelerate
>> another task p.
>>
>> What's this typical usecase?
> 
> The semantics for yield_task under CFS are not well-defined.  With our
> CFS hierarchy, we cannot easily just push a yielded task to the end of
> a runqueue.  And, we don't want to play games with artificially
> increasing vruntime, as this results in potentially high latency for a
> yielded task to get back on CPU.
> 
> I'd interpret a task that calls yield as saying "I can run, but try to
> run something else."  I'd agree that this patch is imperfect in
> achieving this, but I think it is better than the current
> implementation (or at least, less broken).  Currently, a side-effect
> of calling yield is that all other tasks in the same hierarchy get
> skipped as well.  This is almost certainly not what the user
> expects/wants.  It is true that if a yielded task has no other tasks
> in its cgroup on the same CPU, we will potentially end up just picking
> the yielded task again.  But this should be OK; a yielded task should
> be able to continue making forward progress.  Any yielded task that
> calls yield again is likely implementing a busy loop, which is an
> improper use of yield anyway.

I see the issue you want to address.

But isn't then the comment in the patch "... a thread in a cgroup wants
to yield CPU to another thread within the same cgroup ..." misleading?

IMHO, a task can't yield to another task. It can only relinquish the CPU.

Someone could argue that in the current implementation, the task which
calls yield acts on behalf of all the tasks in its taskgroup hierarchy.
But this can have issues as you pointed out.

[...]