Date:   Mon, 27 Feb 2023 22:07:35 +0000
From:   Qais Yousef <qyousef@layalina.io>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Xuewen Yan <xuewen.yan@unisoc.com>, vincent.guittot@linaro.org,
        mingo@redhat.com, juri.lelli@redhat.com, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
        bristot@redhat.com, vschneid@redhat.com,
        linux-kernel@vger.kernel.org, ke.wang@unisoc.com,
        zhaoyang.huang@unisoc.com
Subject: Re: [RFC PATCH] sched/fair: update the vruntime to be max vruntime
 when yield
Message-ID: <20230227220735.3kaytmtt53uoegq7@airbuntu>
References: <20230222080314.2146-1-xuewen.yan@unisoc.com>
 <Y/zO8WZV2kvcU78b@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <Y/zO8WZV2kvcU78b@hirez.programming.kicks-ass.net>
Precedence: bulk

On 02/27/23 16:40, Peter Zijlstra wrote:
> On Wed, Feb 22, 2023 at 04:03:14PM +0800, Xuewen Yan wrote:
> > When task call the sched_yield, cfs would set the cfs's skip buddy.
> > If there is no other task call the sched_yield syscall, the task would
> > always be skiped when there are tasks in rq. 
> 
> So you have two tasks A) which does sched_yield() and becomes ->skip,
> and B) which is while(1). And you're saying that once A does it's thing,
> B runs forever and starves A?

I read it differently.

I understood that there are multiple tasks.

If Task A becomes ->skip; then it seems other tasks will continue to be picked
instead. Until another task B calls sched_yield() and become ->skip, then Task
A is picked but with wrong vruntime causing it to run for multiple ticks (my
interpretation of 'always run' below).

There are no while(1) task running IIUC.

> 
> > As a result, the task's
> > vruntime would not be updated for long time, and the cfs's min_vruntime
> > is almost not updated.
> 
> But the condition in pick_next_entity() should ensure that we still pick
> ->skip when it becomes too old. Specifically, when it gets more than
> wakeup_gran() behind.

I am not sure I can see it either. Maybe __pick_first_entity() doesn't return
the skipped one, or for some reason vdiff for second is almost always
< wakeup_gran()?

> 
> > When this scenario happens, when the yield task had wait for a long time,
> > and other tasks run a long time, once there is other task call the sched_yield,
> > the cfs's skip_buddy is covered, at this time, the first task can run normally,
> > but the task's vruntime is small, as a result, the task would always run,
> > because other task's vruntime is big. This would lead to other tasks can not
> > run for a long time.

The error seems that when Task A finally runs - it consumes more than its fair
bit of sched_slice() as it looks it was starved.

I think the question is why it was starved? Can you shed some light Xuewen?

My attempt to help to clarify :) I have read this just like you.

FWIW I have seen a report of something similar, but I didn't managed to
reproduce and debug (mostly due to ENOBANDWIDTH); and not sure if the details
are similar to what Xuewen is seeing. But there was a task starving for
multiple ticks - RUNNABLE but never RUNNING in spite of other tasks getting
scheduled in instead multiple times. ie: there was a task RUNNING for most of
the time, and I could see it preempted by other tasks multiple time, but not by
the starving RUNNABLE task that is hung on the rq. It seems to be vruntime
related too but speculating here.


Cheers

--
Qais Yousef

> 
> I'm not seeing how this could happen, it should never get behind that
> far.
> 
> Additionally, check_preempt_tick() will explicitly clear the buddies
> when it finds the current task has consumed it's ideal slice.
> 
> I really cannot see how your scenario can happen.
> 
> > In order to mitigate this, update the yield_task's vruntime to be cfs's max vruntime.
> > This way, the cfs's min vruntime can be updated as the process running.
> 
> This is a bad solution, SCHED_IDLE tasks have very low weight and can be
> shot really far to the right, leading to other trouble.
>