MIME-Version: 1.0
References: <1640262603-19339-1-git-send-email-CruzZhao@linux.alibaba.com>
 <1640262603-19339-3-git-send-email-CruzZhao@linux.alibaba.com>
 <CABk29NsP+sMQPRwS2e3zoeBsX1+p2aevFFO+i9GdB5VQ0ujEbA@mail.gmail.com>
 <8be4679f-632b-97e5-9e48-1e1a37727ddf@linux.alibaba.com> <CABk29Nv4OXnNz5-ZdYmAE8o0YpmhkbH=GooksaKYY7n0YYUQxg@mail.gmail.com>
 <3dc03eec-e88c-f886-efd5-81162350f12c@linux.alibaba.com>
In-Reply-To: <3dc03eec-e88c-f886-efd5-81162350f12c@linux.alibaba.com>
From:   Josh Don <joshdon@google.com>
Date:   Thu, 6 Jan 2022 12:03:21 -0800
Message-ID: <CABk29Nt9TMyQJWSYRzWMmmHpT5z85npganZKAF9vkBqJWDhx6Q@mail.gmail.com>
Subject: Re: [PATCH 2/2] sched/core: Uncookied force idle accounting per cpu
To:     cruzzhao <cruzzhao@linux.alibaba.com>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Benjamin Segall <bsegall@google.com>,
        Mel Gorman <mgorman@suse.de>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Alexey Dobriyan <adobriyan@gmail.com>,
        Eric Dumazet <edumazet@google.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Jan 6, 2022 at 4:05 AM cruzzhao <cruzzhao@linux.alibaba.com> wrote:
>
>
> =E5=9C=A8 2022/1/6 =E4=B8=8A=E5=8D=884:59, Josh Don =E5=86=99=E9=81=93:
>
> It's a good idea to combine them into a single sum. I separated them in
> order to be consistent with the task accounting and for easy to understan=
d.
> As for change the task accounting, I've tried but I haven't found a
> proper method to do so. I've considered the following methods:
> 1. Account the uncookie'd force idle time to the uncookie'd task, but
> it'll be hard to trace the uncookie'd task.

Not sure what you mean there, I think you just need to add

--- a/kernel/sched/core_sched.c
+++ b/kernel/sched/core_sched.c
@@ -294,7 +294,7 @@ void sched_core_account_forceidle(struct rq *rq)
                rq_i =3D cpu_rq(i);
                p =3D rq_i->core_pick ?: rq_i->curr;

-               if (!p->core_cookie)
+               if (p =3D=3D rq_i->idle)
                        continue;

                __schedstat_add(p->stats.core_forceidle_sum, delta)

> 2. Account the uncookie'd force idle time to the cookie'd task in the
> core_tree of the core, but it will cost a lot on traversing the core_tree=
.
>
> Many thanks for suggestions.
> Best,
> Cruz Zhao
>
> > Why do you need this separated out into two fields then? Could we just
> > combine the uncookie'd and cookie'd forced idle into a single sum?
> >
> > IMO it is fine to account the forced idle from uncookie'd tasks, but
> > we should then also change the task accounting to do the same, for
> > consistency.