Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <1646917125-20038-1-git-send-email-wangqing@vivo.com>
 <CAKfTPtAvbpUUaOqP3gmOT7fLk8-7v70LzBUiQ-vgDvc7ZZujag@mail.gmail.com>
 <SL2PR06MB30828CF9FF2879AFC9DC53D2BD0C9@SL2PR06MB3082.apcprd06.prod.outlook.com>
 <CAKfTPtBM9WBEgP_PpRHNaATvp9FrBeV2NXhg_EAp4xYtBZYCWQ@mail.gmail.com> <SL2PR06MB30826C314139509EC4778C1BBD0C9@SL2PR06MB3082.apcprd06.prod.outlook.com>
In-Reply-To: <SL2PR06MB30826C314139509EC4778C1BBD0C9@SL2PR06MB3082.apcprd06.prod.outlook.com>
From:   Vincent Guittot <vincent.guittot@linaro.org>
Date:   Fri, 11 Mar 2022 09:25:45 +0100
Message-ID: <CAKfTPtA=dy4M2OK+GqvPDo9D8YBZAHbEpM-YPVZEyEUjEJKU5g@mail.gmail.com>
Subject: Re: [PATCH] sched: topology: make cache topology separate from cpu topology
To:     =?UTF-8?B?546L5pOO?= <wangqing@vivo.com>
Cc:     Catalin Marinas <Catalin.Marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        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>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, 11 Mar 2022 at 09:18, =E7=8E=8B=E6=93=8E <wangqing@vivo.com> wrote:
>
>
> >>
> >>
> >> >On Thu, 10 Mar 2022 at 13:59, Qing Wang <wangqing@vivo.com> wrote:
> >> >>
> >> >> From: Wang Qing <wangqing@vivo.com>
> >> >>
> >> >> Some architectures(e.g. ARM64), caches are implemented below:
> >> >> cluster:                     ****** cluster 0 *****      ****** clu=
ster 1 *****
> >> >> core:                         0      1          2      3          4=
      5           6      7
> >> (add cache level 1)        c0    c1        c2    c3         c4    c5  =
       c6    c7
> >> >> cache(Leveln):         **cache0**  **cache1**  **cache2**  **cache3=
**
> >> (add cache level 3)        *************share level 3 cache **********=
*****
> >> >> sd_llc_id(current):     0      0          0      0          4      =
4           4      4
> >> >> sd_llc_id(should be): 0      0          2      2          4      4 =
          6      6
> >> >>
> >> Here, n always be 2 in ARM64, but others are also possible.
> >> core[0,1] form a complex(ARMV9),  which share L2 cache, core[2,3] is t=
he same.
> >>
> >> >> Caches and cpus have different topology, this causes cpus_share_cac=
he()
> >> >> return the wrong value, which will affect the CPU load balance.
> >> >>
> >> >What does your current scheduler topology  look like?
> >> >
> >> >For CPU 0 to 3, do you have the below ?
> >> >DIE [0     -     3] [4-7]
> >> >MC  [0] [1] [2] [3]
> >>
> >> The current scheduler topology consistent with CPU topology:
> >> DIE  [0-7]
> >> MC  [0-3] [4-7]  (SD_SHARE_PKG_RESOURCES)
> >> Most Android phones have this topology.
> >> >
> >> >But you would like something like below for cpu 0-1 instead ?
> >> >DIE [0     -     3] [4-7]
> >> >CLS [0 - 1] [2 - 3]
> >> >MC  [0] [1]
> >> >
> >> >with SD_SHARE_PKG_RESOURCES only set to MC level ?
> >>
> >> We don't change the current scheduler topology, but the
> >> cache topology should be separated like below:
> >
> >The scheduler topology is not only cpu topology but a mixed of cpu and
> >cache/memory cache topology
> >
> >> [0-7]                          (shared level 3 cache )
> >> [0-1] [2-3][4-5][6-7]   (shared level 2 cache )
> >
> >So you don't  bother the intermediate cluster level which is even simple=
r.
> >you have to modify generic arch topology so that cpu_coregroup_mask
> >returns the correct cpu mask directly.
> >
> >You will notice a llc_sibling field that is currently used by acpi but
> >not DT to return llc cpu mask
> >
> cpu_topology[].llc_sibling describe the last level cache of whole system,
> not in the sched_domain.
>
> in the above cache topology, llc_sibling is 0xff([0-7]) , it describes

If llc_sibling was 0xff([0-7] on your system, you would have only one level=
:
MC[0-7]

> the L3 cache sibling, but sd_llc_id describes the maximum shared cache
> in sd, which should be [0-1] instead of [0-3].

sd_llc_id describes the last sched_domain with SD_SHARE_PKG_RESOURCES.
If you want llc to be [0-3] make sure that the
sched_domain_topology_level array returns the correct cpumask with
this flag


>
> Thanks,
> Wang