lkml.org 
[lkml]   [2008]   [Aug]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] irq: sparse irqs, fix #2
    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?

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

    I give up. Have a little think about the stack bloat, please.

    btw, whoever wrote that function is in need of a tab key.


    \
     
     \ /
      Last update: 2008-08-14 18:37    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean