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

From: Steven Sistare
Date: Fri Dec 07 2018 - 17:36:30 EST


On 12/7/2018 3:20 PM, Valentin Schneider wrote:
> Hi Steve,
>
> On 06/12/2018 21:28, Steve Sistare wrote:
> [...]
>> @@ -1621,7 +1626,22 @@ static void __sdt_free(const struct cpumask *cpu_map)
>>
>> 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));
>> + int flags = __GFP_ZERO | GFP_KERNEL;
>> + struct sparsemask *mask;
>> +
>> + /*
>> + * Allocate the bitmap if not already allocated. This is called for
>> + * every CPU in the LLC but only allocates once per sd_llc_shared.
>> + */
>> + if (!sds->cfs_overload_cpus) {
>> + mask = sparsemask_alloc_node(nr_cpu_ids, 3, flags, nid);
> ^^^^^^^^^^ ^^^
> (1) (2)
>
> (1): Is this necessary? Wouldn't cpumask_weight(span) suffice?

weight does not work because the ids are not consecutive. As a future
optimization I have a patch that computes a local CPU id within the LLC
and uses that as the bitmap index, and that uses weight. For this first
series I am keeping things simple.

> (2): Could this get a name somewhere? In v3 this was
> SPARSEMASK_DENSITY_DEFAULT, I think it'd make sense to have it in
> sched/sparsemask.c

Darn, suspected someone would like this identifier after I deleted it.

- Steve

>> + if (!mask)
>> + return 1;
>> + sds->cfs_overload_cpus = mask;
>> + }
>>
>> return 0;
>> }
>> @@ -1633,7 +1653,8 @@ static void sd_llc_free(struct sched_domain *sd)
>> if (!sds)
>> return;
>>
>> - /* Free data here. Empty for now. */
>> + sparsemask_free(sds->cfs_overload_cpus);
>> + sds->cfs_overload_cpus = NULL;
>> }
>>
>> static int sd_llc_alloc_all(const struct cpumask *cpu_map, struct s_data *d)
>>