Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200820120025.74460-1-benbjiang@tencent.com> <CAKfTPtABCbEuYf3uup5ZODyPXpUqBjgM8A5gBQqM0cQGxWk5zw@mail.gmail.com>
 <20200820125829.GT2674@hirez.programming.kicks-ass.net> <CAKfTPtBuZOb9-W5sS=DqxcuMFa8Lw=7XqMJ_YrXEgd5zvufYjA@mail.gmail.com>
 <20200820134341.GW2674@hirez.programming.kicks-ass.net> <CAKfTPtC203mVB6QB1APbZ5igavdcwbJsCQH_8K__rhGxi+KXoQ@mail.gmail.com>
In-Reply-To: <CAKfTPtC203mVB6QB1APbZ5igavdcwbJsCQH_8K__rhGxi+KXoQ@mail.gmail.com>
From:   Jiang Biao <benbjiang@gmail.com>
Date:   Sun, 23 Aug 2020 15:33:13 +0800
Message-ID: <CAPJCdBmbTULFE=tS0-VM9aqKmC0b0PFfny6=UiuVZB=hVW996Q@mail.gmail.com>
Subject: Re: [PATCH] sched/fair: avoid vruntime compensation for SCHED_IDLE task
To:     Vincent Guittot <vincent.guittot@linaro.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Jiang Biao <benbjiang@tencent.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi, Vincent and Peter

On Thu, 20 Aug 2020 at 22:09, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Thu, 20 Aug 2020 at 15:44, <peterz@infradead.org> wrote:
> >
> > > That's been said, not compensating the vruntime for a sched_idle task
> > > makes sense for me. Even if that will only help for others task in the
> > > same cfs_rq
> >
> > Yeah, but it is worth the extra pointer chasing and branches?
>
> For that I let Jiang provides figures to show the worthful
Using the following configuration for rt-app,
{
        "tasks" : {
                "task_other" : {
                        "instance" : 1, //only 1 instance to be easy to observe
                        "cpus" : [2],
                        "loop" : 2000,
                        "policy" : "SCHED_OTHER",
                        "run" : -1,  //make normal task 100% running
                        "priority" : 0,
                        "sleep" : 0
                },
                "task_idle" : {
                        "instance" : 1,
                        "cpus" : [2],
                        "loop" : 2000,
                        "policy" : "SCHED_IDLE",
                        "run" : 1, //only run 1us to avoid
blocking(always waiting for running), making check_preempt_wakeup
work(S->R switching)
                        "timer" : { "ref" : "unique2" , "period" :
16000, "mode" : "absolute" }
                }
        },
        "global" : {
                "calibration" : "CPU0",
                "default_policy" : "SCHED_OTHER",
                "duration" : -1
        }
}
without the patch,
          <...>-39771 [002] d.h. 42478.177771: sched_wakeup:
comm=task_idle-1 pid=39772 prio=120 target_cpu=002
           <...>-39771 [002] d... 42478.190437: sched_switch:
prev_comm=task_other-0 prev_pid=39771 prev_prio=120 prev_state=R ==>
next_comm=task_idle-1 next_pid=39772 next_prio=120
           <...>-39771 [002] d.h. 42478.193771: sched_wakeup:
comm=task_idle-1 pid=39772 prio=120 target_cpu=002
           <...>-39771 [002] d... 42478.206438: sched_switch:
prev_comm=task_other-0 prev_pid=39771 prev_prio=120 prev_state=R ==>
next_comm=task_idle-1 next_pid=39772 next_prio=120
           <...>-39771 [002] d.h. 42478.209771: sched_wakeup:
comm=task_idle-1 pid=39772 prio=120 target_cpu=002
           <...>-39771 [002] d... 42478.222438: sched_switch:
prev_comm=task_other-0 prev_pid=39771 prev_prio=120 prev_state=R ==>
next_comm=task_idle-1 next_pid=39772 next_prio=120
           <...>-39771 [002] d.h. 42478.225771: sched_wakeup:
comm=task_idle-1 pid=39772 prio=120 target_cpu=002
           <...>-39771 [002] d... 42478.238438: sched_switch:
prev_comm=task_other-0 prev_pid=39771 prev_prio=120 prev_state=R ==>
next_comm=task_idle-1 next_pid=39772 next_prio=120
*task_idle*  preempts every 12ms because of the compensation.

with the patch,
   task_other-0-27670 [002] d.h. 136785.278059: sched_wakeup:
comm=task_idle-1 pid=27671 prio=120 target_cpu=002
    task_other-0-27670 [002] d... 136785.293623: sched_switch:
prev_comm=task_other-0 prev_pid=27670 prev_prio=120 prev_state=R ==>
next_comm=task_idle-1 next_pid=27671 next_prio=120
    task_other-0-27670 [002] d.h. 136785.294059: sched_wakeup:
comm=task_idle-1 pid=27671 prio=120 target_cpu=002
    task_other-0-27670 [002] d... 136785.317624: sched_switch:
prev_comm=task_other-0 prev_pid=27670 prev_prio=120 prev_state=R ==>
next_comm=task_idle-1 next_pid=27671 next_prio=120
    task_other-0-27670 [002] d.h. 136785.326059: sched_wakeup:
comm=task_idle-1 pid=27671 prio=120 target_cpu=002
    task_other-0-27670 [002] d... 136785.341622: sched_switch:
prev_comm=task_other-0 prev_pid=27670 prev_prio=120 prev_state=R ==>
next_comm=task_idle-1 next_pid=27671 next_prio=120
    task_other-0-27670 [002] d.h. 136785.342059: sched_wakeup:
comm=task_idle-1 pid=27671 prio=120 target_cpu=002
    task_other-0-27670 [002] d... 136785.365623: sched_switch:
prev_comm=task_other-0 prev_pid=27670 prev_prio=120 prev_state=R ==>
next_comm=task_idle-1 next_pid=27671 next_prio=120
*task_idle* preempts every 24 or 16 ms.

This patch could reduce the preempting frequency of task_idle, and
reduce the interference from SCHED_IDLE task.

Thx.
Regards,
Jiang