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: <1634278612-17055-1-git-send-email-huangzhaoyang@gmail.com>
 <YYGV1TxsZXzGXFmx@cmpxchg.org> <CAGWkznEaEEz=m5UmPXRECiizwht7+8Zw_xH9V7Wwyd__10eJDA@mail.gmail.com>
 <CAGWkznFuX=6mSnj7J7=t7et5QO-GB2BKCMRiHoU37jcH9dPhLA@mail.gmail.com>
 <78b3f72b-3fe7-f2e0-0e6b-32f28b8ce777@arm.com> <CAGWkznF_8iBp57BPoQKvG4VuNYep=g+ZxgO7D4e0wMDLipJ8uw@mail.gmail.com>
 <85c81ab7-49ed-aba5-6221-ea6a8f37f8ad@arm.com> <CAB8ipk_0YxWnS-k+HLPnL7DRR1MM+WH-xQfna7jD_+TQ0vKi8Q@mail.gmail.com>
 <05a2e61e-9c55-8f8d-5e72-9854613e53c9@arm.com>
In-Reply-To: <05a2e61e-9c55-8f8d-5e72-9854613e53c9@arm.com>
From:   Xuewen Yan <xuewen.yan94@gmail.com>
Date:   Wed, 10 Nov 2021 13:36:45 +0800
Message-ID: <CAB8ipk-r0LOudU7+ijVGS5mu2stKY8DBZ3BhKfbFcOgZj2VaQA@mail.gmail.com>
Subject: Re: [Resend PATCH] psi : calc cfs task memstall time more precisely
To:     Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc:     Zhaoyang Huang <huangzhaoyang@gmail.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Michal Hocko <mhocko@kernel.org>,
        Vladimir Davydov <vdavydov.dev@gmail.com>,
        Zhaoyang Huang <zhaoyang.huang@unisoc.com>,
        "open list:MEMORY MANAGEMENT" <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        xuewen.yan@unisoc.com, Ke Wang <Ke.Wang@unisoc.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Dietmar
On Tue, Nov 9, 2021 at 5:43 PM Dietmar Eggemann
<dietmar.eggemann@arm.com> wrote:
>
> On 08/11/2021 09:49, Xuewen Yan wrote:
> > Hi Dietmar
> >
> > On Sat, Nov 6, 2021 at 1:20 AM Dietmar Eggemann
> > <dietmar.eggemann@arm.com> wrote:
> >>
> >> On 05/11/2021 06:58, Zhaoyang Huang wrote:
> >>>> I don't understand the EAS (probably asymmetric CPU capacity is meant
> >>>> here) angle of the story. Pressure on CPU capacity which is usable for
> >>>> CFS happens on SMP as well?
> >>>  Mentioning EAS here mainly about RT tasks preempting small CFS tasks
> >>> (big CFS tasks could be scheduled to big core), which would introduce
> >>> more proportion of preempted time within PSI_MEM_STALL than SMP does.
> >>
> >> What's your CPU layout? Do you have the little before the big CPUs? Like
> >> Hikey 960?
>
> [...]
>
> >> And I guess rt class prefers lower CPU numbers hence you see this?
> >>
> > our CPU layout is:
> > xuewen.yan:/ # cat /sys/devices/system/cpu/cpu*/cpu_capacity
> > 544
> > 544
> > 544
> > 544
> > 544
> > 544
> > 1024
> > 1024
> >
> > And in our platform, we use the kernel in mobile phones with Android.
> > And we prefer power, so we prefer the RT class to run on little cores.
>
> Ah, OK, out-of-tree extensions.
>
> [...]
>
> >>>>>>>> +     if (current->in_memstall)
> >>>>>>>> +             growth_fixed = div64_ul((1024 - rq->avg_rt.util_avg - rq->avg_dl.util_avg
> >>>>>>>> +                                     - rq->avg_irq.util_avg + 1) * growth, 1024);
> >>>>>>>> +
> >>>>
> >>>> We do this slightly different in scale_rt_capacity() [fair.c]:
> >>>>
> >>>> max = arch_scale_cpu_capacity(cpu_of(rq) /* instead of 1024 to support
> >>>>                                             asymmetric CPU capacity */
> >>> Is it possible that the SUM of rqs' util_avg large than
> >>> arch_scale_cpu_capacity because of task migration things?
> >>
> >> I assume you meant if the rq (cpu_rq(CPUx)) util_avg sum (CFS, RT, DL,
> >> IRQ and thermal part) can be larger than arch_scale_cpu_capacity(CPUx)?
> >>
> >> Yes it can.
> >>
> >> Have a lock at
> >>
> >> effective_cpu_util(..., max, ...) {
> >>
> >>   if (foo >= max)
> >>     return max;
> >>
> >> }
> >>
> >> Even the CFS part (cpu_rq(CPUx)->cfs.avg.util_avg) can be larger than
> >> the original cpu capacity (rq->cpu_capacity_orig).
> >>
> >> Have a look at cpu_util(). capacity_orig_of(CPUx) and
> >> arch_scale_cpu_capacity(CPUx) both returning rq->cpu_capacity_orig.
> >>
> >
> > Well, your means is we should not use the 1024 and should use the
> > original cpu capacity?
> > And maybe use the "sched_cpu_util()" is a good choice just like this:
> >
> > +       if (current->in_memstall)
> > +               growth_fixed = div64_ul(cpu_util_cfs(rq) * growth,
> > sched_cpu_util(rq->cpu, capacity_orig_of(rq->cpu)));
>
> Not sure about this. In case util_cfs=0 you would get scale=0.

Yes , we should consider it. In addition, it also should be considered
when util_cfs > capacity_orig because of the UTIL_EST......

>
> IMHO, you need
>
> cap      = rq->cpu_capacity
> cap_orig = rq->cpu_capacity_orig
>
> scale = (cap * X) / cap_orig
>
> or if the update of these rq values happens to infrequently for you then
> you have to calc the pressure evey time. Something like:
>
> pressure = cpu_util_rt(rq) + cpu_util_dl(rq)
>
> irq = cpu_util_irq(rq)
>
> if (irq >= cap_orig)
>     pressure = cap_orig
> else
>     pressure = scale_irq_capacity(pressure, irq, cap_orig)
>     pressure += irq
>
> scale = ((cap_orig - pressure) * X) / cap_orig

Why rescale the util there, the sched_cpu_util() would invoke the
effective_cpu_util(), and I don't think it's necessary to rescale it.

Thanks!