Re: [PATCH v3 03/10] sched/topology: Provide cfs_overload_cpus bitmap

From: Valentin Schneider
Date: Fri Nov 09 2018 - 12:38:42 EST


Hi Steve,

On 09/11/2018 12:50, Steve Sistare wrote:
[...]
> @@ -482,6 +484,10 @@ static void update_top_cache_domain(int cpu)
> dirty_sched_domain_sysctl(cpu);
> destroy_sched_domains(tmp);
>
> + sd = highest_flag_domain(cpu, SD_SHARE_PKG_RESOURCES);
> + cfs_overload_cpus = (sd ? sd->shared->cfs_overload_cpus : NULL);
> + rcu_assign_pointer(rq->cfs_overload_cpus, cfs_overload_cpus);
> +

Why not do this in update_top_cache_domain() where we also look for the
highest SD_SHARE_PKG_RESOURCES and setup shortcut pointers?

> update_top_cache_domain(cpu);
> }
>
> @@ -1619,9 +1625,19 @@ static void __sdt_free(const struct cpumask *cpu_map)
> }
> }
>
> +#define ZALLOC_MASK(maskp, nelems, node) \
> + (!*(maskp) && !zalloc_sparsemask_node(maskp, nelems, \
> + SPARSEMASK_DENSITY_DEFAULT, \
> + GFP_KERNEL, node)) \
> +
> static int sd_llc_alloc(struct sched_domain *sd)
> {
> - /* Allocate sd->shared data here. Empty for now. */
> + struct sched_domain_shared *sds = sd->shared;
> + struct cpumask *span = sched_domain_span(sd);
> + int nid = cpu_to_node(cpumask_first(span));
> +
> + if (ZALLOC_MASK(&sds->cfs_overload_cpus, nr_cpu_ids, nid))

Mmm so this is called once on every CPU, but the !*(maskp) check in the
macro makes it so there is only one allocation per sd_llc_shared.

I wouldn't mind having that called out in a comment, or having the
pointer check done explicitly outside of the macro.

[...]