RE: [PATCH v6] lib/group_cpus: Snapshot cluster masks to keep grouping hotplug invariant

[Guo, Wangyang]

Why not use original topology_sibling_cpumask(cpu) if snapshot not
available?
> -		siblmsk = topology_sibling_cpumask(cpu);
> +		siblmsk = sibling_snapshot ? sibling_snapshot[cpu] : cpumask_of(cpu);


> - * Return: true if success, false otherwise.
> + * Return: 1 if successful, 0 if inapplicable, or -ENOMEM on allocation failure.
>   */
> -static bool __try_group_cluster_cpus(unsigned int ncpus,
> +static int __try_group_cluster_cpus(unsigned int ncpus,

If use ncluster as ret: >= 1 successful, 0 inapplicable, and -ENOMEM for
error.
the code should become simpler.
>  	ncluster = alloc_cluster_groups(ncpus, ngroups, node_cpumask, nmsk,
>  					&clusters, &cluster_groups);
>
> -	if (ncluster == 0)
> +	ret = ncluster;
> +	if (ncluster <= 0)
>  		goto fail_no_clusters;
>
>  	for (i = 0; i < ncluster; i++) {
> @@ -392,12 +477,11 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
>  		assign_cpus_to_groups(nc, nmsk, nv, masks, curgrp, last_grp);
>  	}
>
> -	ret = true;
> +	ret = 1;
>  	kfree(cluster_groups);
>  	kfree(clusters);
>   fail_no_clusters:
>  	free_cpumask_var(nmsk);
> - fail_nmsk_alloc:
              return ncluster;
>  	return ret;
>  }


> @@ -442,6 +526,7 @@ static int __group_cpus_evenly(unsigned int startgrp, unsigned int numgrps,
> -		if (__try_group_cluster_cpus(ncpus, nv->ngroups, nmsk,
> -					     masks, &curgrp, last_grp)) {
> +		ret = __try_group_cluster_cpus(ncpus, nv->ngroups, nmsk,
> +					       masks, &curgrp, last_grp);
> +		if (ret < 0) {
> +			kfree(node_groups);
> +			return ret;
> +		}
> +		if (ret) {
>  			done += nv->ngroups;
>  			continue;
>  		}

There is duplication for "done += nv->ngroups", my suggestion:
    If (ret < 0) { kfree(node_groups); return ret;}
    elIf (!ret) assign_cpus_to_groups(ncpus, nmsk, nv, masks, &curgrp, last_grp);
    done += nv->ngroups;
}  // end of "for" loop