Messages in this thread | | | From | 王擎 <> | Subject | [PATCH] arch_topology: support parsing cache topology from DT | Date | Fri, 8 Apr 2022 09:25:59 +0000 |
| |
>> >> >> 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
| |