On 11/03/2025 13:34, Waiman Long wrote: > On 3/11/25 7:59 AM, Juri Lelli wrote: >> On 10/03/25 20:16, Waiman Long wrote: >>> On 3/10/25 3:18 PM, Waiman Long wrote: >>>> On 3/10/25 2:54 PM, Dietmar Eggemann wrote: >>>>> On 10/03/2025 10:37, Juri Lelli wrote: >>>>>> Rebuilding of root domains accounting information (total_bw) is >>>>>> currently broken on some cases, e.g. suspend/resume on aarch64. >>>>>> Problem >>>>> Nit: Couldn't spot any arch dependency here. I guess it was just >>>>> tested >>>>> on Arm64 platforms so far. >>>>> >>>>> [...] >>>>> >>>>>> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c >>>>>> index 44093339761c..363ad268a25b 100644 >>>>>> --- a/kernel/sched/topology.c >>>>>> +++ b/kernel/sched/topology.c >>>>>> @@ -2791,6 +2791,7 @@ void partition_sched_domains_locked(int >>>>>> ndoms_new, cpumask_var_t doms_new[], >>>>>> ndoms_cur = ndoms_new; >>>>>> update_sched_domain_debugfs(); >>>>>> + dl_rebuild_rd_accounting(); >>>>> Won't dl_rebuild_rd_accounting()'s lockdep_assert_held(&cpuset_mutex) >>>>> barf when called via cpuhp's: >>>>> >>>>> sched_cpu_deactivate() >>>>> >>>>> cpuset_cpu_inactive() >>>>> >>>>> partition_sched_domains() >>>>> >>>>> partition_sched_domains_locked() >>>>> >>>>> dl_rebuild_rd_accounting() >>>>> >>>>> ? >> Good catch. Guess I didn't notice while testing with LOCKDEP as I was >> never able to hit this call path on my systems. >> >>>> Right. If cpuhp_tasks_frozen is true, partition_sched_domains() will be >>>> called without holding cpuset mutex. >>>> >>>> Well, I think we will need an additional wrapper in cpuset.c that >>>> acquires the cpuset_mutex first before calling >>>> partition_sched_domains() >>>> and use the new wrapper in these cases. >>> Actually, partition_sched_domains() is called with the special >>> arguments (1, >>> NULL, NULL) to reset the domain to a single one. So perhaps something >>> like >>> the following will be enough to avoid this problem. >> I think this would work, as we will still rebuild the accounting after >> last CPU comes back from suspend. The thing I am still not sure about is >> what we want to do in case we have DEADLINE tasks around, since with >> this I belive we would be ignoring them and let suspend proceed. > > That is the current behavior. You can certainly create a test case to > trigger such condition and see what to do about it. Alternatively, you > can document that and come up with a follow-up patch later on. But don't we rely on that partition_sched_domains_locked() calls dl_rebuild_rd_accounting() even in the reset_domain=1 case? Testcase: suspend/resume on Arm64 big.LITTLE cpumask=[LITTLE][big]=[0,3-5][1-2] plus cmd line option 'isolcpus=3,4'. with Waiman's snippet: https://lkml.kernel.org/r/fd4d6143-9bd2-4a7c-80dc-1e19e4d1b2d1@xxxxxxxxxx ... [ 234.831675] --- > partition_sched_domains_locked() reset_domain=1 [ 234.835966] psci: CPU4 killed (polled 0 ms) [ 234.838912] Error taking CPU3 down: -16 [ 234.838952] Non-boot CPUs are not disabled [ 234.838986] Enabling non-boot CPUs ... ... IIRC, that's the old DL accounting issue.
