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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1340315496.3696.87.camel@sbsiddha-desk.sc.intel.com>
Date:	Thu, 21 Jun 2012 14:51:35 -0700
From:	Suresh Siddha <suresh.b.siddha@...el.com>
To:	Alexander Gordeev <agordeev@...hat.com>
Cc:	Ingo Molnar <mingo@...nel.org>, yinghai@...nel.org,
	linux-kernel@...r.kernel.org, x86@...nel.org, gorcunov@...nvz.org
Subject: Re: [PATCH 2/2] x86, x2apic: limit the vector reservation to the
 user specified mask

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/