Re: [PATCH 2/2] x86, x2apic: limit the vector reservation to the user specified mask

[Suresh Siddha <suresh.b.siddha@intel.com>]

On Thu, 2012-06-21 at 11:04 +0200, Alexander Gordeev wrote:
> >  static int
> >  __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
> >  {
> > @@ -1126,10 +1142,9 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
> >  	old_vector = cfg->vector;
> >  	if (old_vector) {
> >  		cpumask_and(tmp_mask, mask, cpu_online_mask);
> > -		if (cpumask_subset(tmp_mask, cfg->domain)) {
> > -			free_cpumask_var(tmp_mask);
> > -			return 0;
> > -		}
> > +		apic->vector_allocation_domain(mask, tmp_mask, cfg->domain);
> > +		if (cpumask_subset(tmp_mask, cfg->domain))
> > +			return cleanup_unused_subset(tmp_mask, cfg);
> 
> ...but if you decide to leave cleanup_unused_subset() then cpumask_subset()
> check is better to move inside the function while free_cpumask_var() move
> out.

Removed the function and the above hunk completely in the next version.

> 
> >  	}
> >  
> >  	/* Only try and allocate irqs on cpus that are present */
> > @@ -1137,15 +1152,12 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
> >  	cpumask_clear(cfg->old_domain);
> >  	cpu = cpumask_first_and(mask, cpu_online_mask);
> >  	while (cpu < nr_cpu_ids) {
> > -		int new_cpu;
> > -		int vector, offset;
> > +		int new_cpu, vector, offset;
> >  
> > -		apic->vector_allocation_domain(cpu, tmp_mask);
> > -
> > -		if (cpumask_subset(tmp_mask, cfg->domain)) {
> > -			free_cpumask_var(tmp_mask);
> > -			return 0;
> > -		}
> > +		cpumask_copy(tmp_mask, cpumask_of(cpu));
> > +		apic->vector_allocation_domain(mask, tmp_mask, cfg->domain);
> 
> I might be missing the point.. in the two lines above you copy a cpumask to
> tmp_mask, then scan it in vector_allocation_domain() just to find the cpu
> which you already know. Why not just pass cpu rather then cpumask_of(cpu)?

One of my earlier experiments was using cfg->domain internally in the
vector_allocation_domain() and didn't want to use to many arguments.
Also 'cpu' argument for the first hunk above was not making sense.

Anyhow, this is all cleaned up now in the next version of the patchset.

thanks,
suresh