Re: [PATCH V6 2/2] genirq/affinity: Spread vectors on node according to nr_cpu ratio

[Ming Lei <ming.lei@redhat.com>]

On Mon, Aug 19, 2019 at 04:02:21PM +0200, Thomas Gleixner wrote:
> On Mon, 19 Aug 2019, Ming Lei wrote:
> > On Mon, Aug 19, 2019 at 03:13:58PM +0200, Thomas Gleixner wrote:
> > > On Mon, 19 Aug 2019, Ming Lei wrote:
> > > 
> > > > Cc: Jon Derrick <jonathan.derrick@intel.com>
> > > > Cc: Jens Axboe <axboe@kernel.dk>
> > > > Reported-by: Jon Derrick <jonathan.derrick@intel.com>
> > > > Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>
> > > > Reviewed-by: Keith Busch <kbusch@kernel.org>
> > > 
> > > This version is sufficiently different from the previous one, so I do not
> > > consider the reviewed-by tags still being valid and meaningful. Don't
> > > include them unless you just do cosmetic changes.
> > 
> > Fine.
> > 
> > However, the V6 only change isn't big, just for addressing the un-initialized
> > warning, and the change is only done on function of irq_build_affinity_masks().
> 
> They are not trivial either:
> 
>  affinity.c |   28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> --- a/kernel/irq/affinity.c
> +++ b/kernel/irq/affinity.c
> @@ -339,7 +339,7 @@ static int irq_build_affinity_masks(unsi
>  				    unsigned int firstvec,
>  				    struct irq_affinity_desc *masks)
>  {
> -	unsigned int curvec = startvec, nr_present, nr_others;
> +	unsigned int curvec = startvec, nr_present = 0, nr_others = 0;
>  	cpumask_var_t *node_to_cpumask;
>  	cpumask_var_t nmsk, npresmsk;
>  	int ret = -ENOMEM;
> @@ -354,19 +354,17 @@ static int irq_build_affinity_masks(unsi
>  	if (!node_to_cpumask)
>  		goto fail_npresmsk;
>  
> -	ret = 0;
>  	/* Stabilize the cpumasks */
>  	get_online_cpus();
>  	build_node_to_cpumask(node_to_cpumask);
>  
>  	/* Spread on present CPUs starting from affd->pre_vectors */
> -	nr_present = __irq_build_affinity_masks(curvec, numvecs,
> -						firstvec, node_to_cpumask,
> -						cpu_present_mask, nmsk, masks);
> -	if (nr_present < 0) {
> -		ret = nr_present;
> +	ret = __irq_build_affinity_masks(curvec, numvecs, firstvec,
> +					 node_to_cpumask, cpu_present_mask,
> +					 nmsk, masks);
> +	if (ret < 0)
>  		goto fail_build_affinity;
> -	}
> +	nr_present = ret;
>  
>  	/*
>  	 * Spread on non present CPUs starting from the next vector to be
> @@ -379,16 +377,16 @@ static int irq_build_affinity_masks(unsi
>  	else
>  		curvec = firstvec + nr_present;
>  	cpumask_andnot(npresmsk, cpu_possible_mask, cpu_present_mask);
> -	nr_others = __irq_build_affinity_masks(curvec, numvecs,
> -					       firstvec, node_to_cpumask,
> -					       npresmsk, nmsk, masks);
> -	if (nr_others < 0)
> -		ret = nr_others;
> +	ret = __irq_build_affinity_masks(curvec, numvecs, firstvec,
> +					 node_to_cpumask, npresmsk, nmsk,
> +					 masks);
> +	if (ret >= 0)
> +		nr_others = ret;
>  
>   fail_build_affinity:
>  	put_online_cpus();
>  
> -	if (min(nr_present, nr_others) >= 0)
> +	if (ret >= 0)
>  		WARN_ON(nr_present + nr_others < numvecs);
>  
>  	free_node_to_cpumask(node_to_cpumask);
> @@ -398,7 +396,7 @@ static int irq_build_affinity_masks(unsi
>  
>   fail_nmsk:
>  	free_cpumask_var(nmsk);
> -	return ret;
> +	return ret < 0 ? ret : 0;
>  }
>  
>  static void default_calc_sets(struct irq_affinity *affd, unsigned int affvecs)

Hi Keith & Jon,

Could you review the above V6 extra change so that we can move on?

BTW, in-balanced numa nodes can be made easily via passing 'possible_cpus=N'.


Thanks, 
Ming