Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v4 03/10] sched/topology: Provide cfs_overload_cpus bitmap
To:     Steve Sistare <steven.sistare@oracle.com>, mingo@redhat.com,
        peterz@infradead.org
Cc:     subhra.mazumdar@oracle.com, dhaval.giani@oracle.com,
        daniel.m.jordan@oracle.com, pavel.tatashin@microsoft.com,
        matt@codeblueprint.co.uk, umgwanakikbuti@gmail.com,
        riel@redhat.com, jbacik@fb.com, juri.lelli@redhat.com,
        vincent.guittot@linaro.org, quentin.perret@arm.com,
        linux-kernel@vger.kernel.org
References: <1544131696-2888-1-git-send-email-steven.sistare@oracle.com>
 <1544131696-2888-4-git-send-email-steven.sistare@oracle.com>
From:   Valentin Schneider <valentin.schneider@arm.com>
Message-ID: <9253783d-d53f-9587-61eb-2505bca5fc0c@arm.com>
Date:   Fri, 7 Dec 2018 20:20:24 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <1544131696-2888-4-git-send-email-steven.sistare@oracle.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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?

(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

> +		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)
>