Re: [RFC 09/13] genapic: reduce stack pressuge in io_apic.c step 1 temp cpumask_ts

Previous thread: [RFC 07/13] sched: Reduce stack size requirements in kernel/sched.c by Mike Travis on Saturday, September 6, 2008 - 4:50 pm. (20 messages)

Next thread: [RFC 06/13] genapic: use get_cpumask_var operations for allbutself cpumask_ts by Mike Travis on Saturday, September 6, 2008 - 4:50 pm. (1 message)
From: Mike Travis
Date: Saturday, September 6, 2008 - 4:50 pm

* Step 1 of cleaning up io_apic.c removes local cpumask_t variables
    from the stack.

  - Method 1: remove unnecessary "extra" cpumask variables.

  - Method 2: use for_each_online_cpu_mask_nr() to logically AND
              the passed in mask with cpu_online_map, eliminating
	      the need for a temp cpumask variable.

  - Method 3: use get_cpumask_var variables where possible. The current
    assignment of temp variables is:
	

    /*
     * Temporary cpumask variables
     *
     * (XXX - would be _MUCH_ better as a "stack" of temp cpumasks.)
     *
     * level 4:
     *     irq_complete_move()
     *     check_timer()
     *     msi_compose_msg()
     *     set_msi_irq_affinity()
     *     ir_set_msi_irq_affinity()
     *     dmar_msi_set_affinity()
     *     set_ht_irq_affinity()
     *     arch_setup_ht_irq()
     *     setup_ioapic_dest()
     *
     * level 3:
     *     set_ioapic_affinity_irq()
     *     setup_IO_APIC_irq()
     *     migrate_ioapic_irq()
     *
     * level 2:
     *     create_irq_nr()
     *
     * level 1:
     *     __assign_irq_vector()
     *     setup_timer_IRQ0_pin()
     */

  * Addition of temp cpumask variables for the "target" of TARGET_CPUS
    is in preparation of changing the TARGET_CPUS for x86_64.  I've
    kept those changes here to document which routines get which temp
    cpumask variables.

  * Total stack size savings are in the last step.

Applies to linux-2.6.tip/master.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 arch/x86/kernel/io_apic.c |  268 ++++++++++++++++++++++++++++++----------------
 1 file changed, 175 insertions(+), 93 deletions(-)

--- linux-2.6.tip.orig/arch/x86/kernel/io_apic.c
+++ linux-2.6.tip/arch/x86/kernel/io_apic.c
@@ -41,6 +41,7 @@
 #endif
 #include <linux/bootmem.h>
 #include <linux/dmar.h>
+#include <linux/cpumask_ptr.h>
 
 #include <asm/idle.h>
 #include <asm/io.h>
@@ -93,6 +94,39 @@ int mp_bus_id_to_type[MAX_MP_BUSSES];
 
 ...
From: Andi Kleen
Date: Monday, September 8, 2008 - 4:01 am

Sorry that patch seems incredibly messy. Global variables
and a tricky ordering and while it's at least commented it's still a mess
and maintenance unfriendly.

Also I think set_affinity is the only case where a truly arbitary cpu
mask can be passed in anyways. And it's passed in from elsewhere. 

The other cases generally just want to handle a subset of CPUs which
are nearby. How about you define a new cpumask like type that 
consists of a start/stop CPU and a mask for that range only 
and is not larger than a few words?

I think with that the nearby assignments could be handled 
reasonably cleanly with arguments and local variables.

And I suspect with some restructuring set_affinity could
be also made to support such a model.

-Andi
--

From: Mike Travis
Date: Monday, September 8, 2008 - 9:03 am

Thanks for the comments.  I did mull over something like this early on
in researching this "cpumask" problem, but the problem of having different
cpumask operators didn't seem worthwhile.  But perhaps for a very limited
use (with very few ops), it would be worthwhile.

But how big to make these?  Variable sized?  Config option?  Should I
introduce some kind of MAX_CPUS_PER_NODE constant?  (NR_CPUS/MAX_NUMNODES
I don't think is the right answer.)

Thanks,
Mike
--

Previous thread: [RFC 07/13] sched: Reduce stack size requirements in kernel/sched.c by Mike Travis on Saturday, September 6, 2008 - 4:50 pm. (20 messages)

Next thread: [RFC 06/13] genapic: use get_cpumask_var operations for allbutself cpumask_ts by Mike Travis on Saturday, September 6, 2008 - 4:50 pm. (1 message)