[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2ea89e07.ac63.1907836ec4b.Coremail.xavier_qy@163.com>
Date: Wed, 3 Jul 2024 18:49:43 +0800 (CST)
From: Xavier <xavier_qy@....com>
To: Michal Koutný <mkoutny@...e.com>, longman@...hat.com
Cc: tj@...nel.org, akpm@...ux-foundation.org, lizefan.x@...edance.com,
hannes@...xchg.org, cgroups@...r.kernel.org,
linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org
Subject: Re:Re: [PATCH-cpuset v10 2/2] cpuset: use Union-Find to optimize
the merging of cpumasks
Hi Michal and Longman,
Please confirm my explanation about cgroup v2 below.
At 2024-07-03 17:40:49, "Michal Koutný" <mkoutny@...e.com> wrote:
>On Wed, Jul 03, 2024 at 02:37:27PM GMT, Xavier <xavier_qy@....com> wrote:
>> @@ -1102,31 +1101,25 @@ static int generate_sched_domains(cpumask_var_t **domains,
>> if (root_load_balance && (csn == 1))
>> goto single_root_domain;
>>
>> - for (i = 0; i < csn; i++)
>> - csa[i]->pn = i;
>> - ndoms = csn;
>> -
>> -restart:
>> - /* Find the best partition (set of sched domains) */
>> - for (i = 0; i < csn; i++) {
>> - struct cpuset *a = csa[i];
>> - int apn = a->pn;
>> -
>> - for (j = 0; j < csn; j++) {
>> - struct cpuset *b = csa[j];
>> - int bpn = b->pn;
>> -
>> - if (apn != bpn && cpusets_overlap(a, b)) {
>> - for (k = 0; k < csn; k++) {
>> - struct cpuset *c = csa[k];
>> + if (!cgrpv2) {
>
>I'm surprised that original code wasn't branched on this on you add it
>here. Why is UF used only for v1 code?
>
In the Patch v6, I explained to Longman that based on his new patch, the overlapping check and
merge operations for cpusets are skipped in the case of cgroup v2. Because for cgroup v2,
doms[i] is merely copied from csa[i] rather than merged.
This needs further confirmation from Longman.
if (cgrpv2) {
for (i = 0; i < ndoms; i++) {
cpumask_copy(doms[i], csa[i]->effective_cpus);
if (dattr)
dattr[i] = SD_ATTR_INIT;
}
goto done;
}
>> + for (i = 0; i < csn; i++)
>> + uf_node_init(&csa[i]->node);
>>
>> - if (c->pn == bpn)
>> - c->pn = apn;
>> - }
>> - ndoms--; /* one less element */
>> - goto restart;
>> + /* Merge overlapping cpusets */
>> + for (i = 0; i < csn; i++) {
>> + for (j = i + 1; j < csn; j++) {
>> + if (cpusets_overlap(csa[i], csa[j]))
>> + uf_union(&csa[i]->node, &csa[j]->node);
>> }
>> }
>> +
>> + /* Count the total number of domains */
>> + for (i = 0; i < csn; i++) {
>> + if (csa[i]->node.parent == &csa[i]->node)
>> + ndoms++;
>
>The naked parent access doesn't hide the UF abstraction well.
>I'd consider uf_find(&csa[i]->node) == &csa[i]->node or a specific
>helper like uf_is_representant(&csa[i]->node).
This can be optimized here. I will change it to the first method you mentioned in the next patch.
Best Regards,
Xavier
Powered by blists - more mailing lists