Messages in this thread | | | From | (Eric W. Biederman) | Date | Thu, 14 Aug 2008 10:03:07 -0700 | Subject | Re: [PATCH] irq: sparse irqs, fix #2 |
| |
Andrew Morton <akpm@linux-foundation.org> writes:
> On Thu, 14 Aug 2008 15:36:52 +0200 Ingo Molnar <mingo@elte.hu> wrote: > >> +static inline cpumask_t vector_allocation_domain(int cpu) >> +{ >> + /* Careful. Some cpus do not strictly honor the set of cpus >> + * specified in the interrupt destination when using lowest >> + * priority interrupt delivery mode. >> + * >> + * In particular there was a hyperthreading cpu observed to >> + * deliver interrupts to the wrong hyperthread when only one >> + * hyperthread was specified in the interrupt desitination. >> + */ >> + cpumask_t domain = { { [0] = APIC_ALL_CPUS, } }; >> + return domain; >> +} > > I haven't looked at callers of this, but... > > Does it need to be allocated on the stack? Local cpumask_t's are a > size problem. Can we build this in .rodata at compile time instead? > > Is this the caller?
Yes.
> > + for_each_cpu_mask(cpu, mask) { > + cpumask_t domain, new_mask; > + int new_cpu; > + int vector; > + > + domain = vector_allocation_domain(cpu); > + cpus_and(new_mask, domain, cpu_online_map); > > If so we could perhaps do > > > static noinline const cpumask_t *vector_allocation_domain(int cpu) > { > /* Careful. Some cpus do not strictly honor the set of cpus > * specified in the interrupt destination when using lowest > * priority interrupt delivery mode. > * > * In particular there was a hyperthreading cpu observed to > * deliver interrupts to the wrong hyperthread when only one > * hyperthread was specified in the interrupt desitination. > */ > static const cpumask_t domain = { { [0] = APIC_ALL_CPUS, } }; > return &domain; > } > > > ... > > + for_each_cpu_mask(cpu, mask) { > + cpumask_t domain, new_mask; > + int new_cpu; > + int vector; > + > + __cpus_and(new_mask, vector_allocation_domain(cpu), > + &cpu_online_map); > > otoh, perhaps this new function is one implementation of > genapic.vector_allocation_domain(), in which case the inlining was > unneeded and misleading.
Likely. Why these things live in header files...
> I give up. Have a little think about the stack bloat, please. > > btw, whoever wrote that function is in need of a tab key.
Unfortunate gradual accreation of functionality.
vector_allocation_domain could perhaps be better named. Round up this cpu to the set of cpus I need to allocate a vector on.
Eric
| |