mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: 王擎 <wangqing@vivo.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH] arch_topology: support parsing cache topology from DT
Date: Fri, 8 Apr 2022 09:25:59 +0000	[thread overview]
Message-ID: <SL2PR06MB30820CCBE8F839B1ADD19DFCBDE99@SL2PR06MB3082.apcprd06.prod.outlook.com> (raw)
In-Reply-To: <20220407103022.fim3cvpnzfngyzkt@bogus>


>> 
>> >> From: wangqing <11112896@bbktel.com>
>> >> 
>> >> When ACPI is not enabled, we can parse cache topolopy from DT:
>> >> *             cpu0: cpu@000 {
>> >> *                     next-level-cache = <&L2_1>;
>> >> *                     L2_1: l2-cache {
>> >> *                              compatible = "cache";
>> >> *                             next-level-cache = <&L3_1>;
>> >> *                      };
>> >> *                     L3_1: l3-cache {
>> >> *                              compatible = "cache";
>> >> *                      };
>> >> *             };
>> >> *
>> >> *             cpu1: cpu@001 {
>> >> *                     next-level-cache = <&L2_1>;
>> >> *             };
>> >> *             cpu2: cpu@002 {
>> >> *                     L2_2: l2-cache {
>> >> *                              compatible = "cache";
>> >> *                             next-level-cache = <&L3_1>;
>> >> *                     };
>> >> *             };
>> >> *
>> >> *             cpu3: cpu@003 {
>> >> *                     next-level-cache = <&L2_2>;
>> >> *             };
>> >> cache_topology hold the pointer describing "next-level-cache", 
>> >> it can describe the cache topology of every level.
>> >> 
>> >> Expand the use of llc_sibling when ACPI is not enabled.
>> >>
>> >
>> >You seem to have posted this patch as part of the series first. One patch
>> >was rejected and then you post this without any history. It confuses if you
>> >don't provide all the background/history.
>>
>> Yes, the series contains several parts, the sched_domain part was rejected
>> temporary. But it has nothing to do with this patch, that's why I took it apart.
>
>That's not correct if you plan to use it there. Currently no users so no need
>to add.
>
>> The background doesn't matter, let's focus on this patch itself.
>>
>
>It depends, some people might find it useful, so better to provide it.
>One can ignore if that is not needed or if they are already aware.
>

I see.

>> >
>> >Having said that, NACK for this patch as it stands. We have
>> >drivers/base/cacheinfo.c which has all the parsing of cache information.
>> >IIRC we already consider LLC but highlight if anything is particularly
>> >missing. I am unable to follow/understand with you commit message. 
>> 
>> cacheinfo.c just describes the properties of the cache, It can't describe
>> the cache topology, some like cpuinfo and cpu topology.
>> 
>
>Not 100% correct. We do have info about sharing there.
>
>> llc_sibling is not used at all if ACPI is not enabled, because llc_id
>> always be -1, and cpu_coregroup_mask() always return the core_sibling.
>>
>
>You can use of_find_last_level_cache or something similar and remove load
>of duplicated code you have in this patch.

First correct my previous mistakes, we could have obtained cache topology
through get_cpu_cacheinfo(cpu)->info_list + index, but its initialization 
is after smp cpumask and llc configuration.

Can we take detect_cache_attributes() before smp prepare, or put it in 
parse_dt_topology()?

>
>> Why not get the cache topology from DT if the arch support GENERIC_ARCH_TOPOLOGY.
>>
>
>Sure but if the intended use is for scheduler, please include relevant people
>as there are quite a few threads around the topic recently and disintegrating
>and throwing patches at random with different set of people is not going to
>help make progress. If this is intended for Arm64 platforms, I suggest to
>keep these 2 in the loop as they are following few other threads and gives
>them full picture of intended use-case.
>
>Vincent Guittot <vincent.guittot@linaro.org>
>Dietmar Eggemann <dietmar.eggemann@arm.com>
>

I will do it if necessary.

Thanks,
Wang

>--
>Regards,
sched_domain_flags_f

  reply	other threads:[~2022-04-08  9:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-06  9:18 Qing Wang
2022-04-06 11:06 ` Sudeep Holla
2022-04-07  3:11   ` 王擎
2022-04-07 10:30     ` Sudeep Holla
2022-04-08  9:25       ` 王擎 [this message]
2022-04-07 19:42 ` Dietmar Eggemann
2022-04-08  2:07   ` 王擎

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SL2PR06MB30820CCBE8F839B1ADD19DFCBDE99@SL2PR06MB3082.apcprd06.prod.outlook.com \
    --to=wangqing@vivo.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=sudeep.holla@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®