Re: [PATCH mask 6/15] nonsmp-cpu-present-map

From: Paul Jackson
Date: Sat May 08 2004 - 06:13:58 EST

Next message: Andrew Morton: "Re: how long does it take to init the scheduler?"
Previous message: Olaf Hering: "Re: how long does it take to init the scheduler?"
Next in thread: Paul Jackson: "Re: [PATCH mask 6/15] nonsmp-cpu-present-map"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Rusty - the last bit of my shenanigans in linux/cpumask.h are below.
I welcome your comment - especially since you were the last one in here.
I think it looks a little better with this patch, but that could be a
matter of taste.

Ashok - I've put lkml on the Cc list. If someone there finds issue
with this, better to see it sooner than later.

Ashok wrote:
> Not sure if this patch is necessary. especially all other maps are treated one way, and
> just for one function fixup we have a differrent treatment for this.

Good questions. What wasn't visible to you at the time you replied was
that I was experimenting with making all three maps (online, present and
possible) treated this 'new' way, removing the big ifdef CONFIG_SMP for
them from linux/cpumask.h, and defining all three of them in
kernel/sched.c in the case CONFIG_SMP is not defined.

This is not a very big change. It's the last step of my planned cpumask
cleanup. Whether you, Rusty or others will find it to be an improvement
or not I don't know.

> Also declaring just one map in sched.c seems like a hack, i moved it to cpu.c just
> to keep the functionality of cpu related masks in relevant files.

The other maps were already defined by my patch set, in the case CONFIG_SMP is
_not_ defined, in kernel/sched.c. Putting cpu_present_map there as well is
consistent with that change.

> Maybe we can use some gcc intrincics to check if this is a constant, and then handle it
> appropriately in the macro definition?

Interesting idea. I tried playing around with this avenue, but didn't come
up with any code that I preferred to what I ended up with, below. If you
would like to offer up some code - I'd be interested to see it.

> I would prefer to keep the decl in cpu.c and ifdef the usage under CONFIG_SMP rather than
> special casing the cpu_present_map alone vs other related cpu maps.

You're quite right that the special casing is not so good. With the
patch below, I went the other way - removing the big #ifdef CONFIG_SMP
from the other two as well.

> I will let Andrew and others decide how to handle it

Well - I suspect Andrew might not spend much of his time arbitrating this.
He usually has more serious stuff to work on. Best if we can agree.

===

Consider the following patch - it goes on top of the sequence that I published
May 6 under the Subject: [PATCH mask 0/15] bitmap and cpumask cleanup.
I have built it for i386 (SMP and not), sparc and ia64 (sn2_defconfig),
and I have booted it on sn2.

To be clear, my 'quilt' series file contains, after the patches for
2.6.6-rc3-mm2, the lines:

pj-fix-1-unifix.patch
pj-fix-2-ashoks-updated-cpuhotplug-6-7.patch
pj-fix-3-ashoks-updated-cpuhotplug-7-7.patch
pj-fix-4-include-mempolicy.patch
pj-fix-5-syscall-return-semicolon.patch
nonsmp-cpu-present-map.patch
mask1-bitmap-cleanup-prep
mask2-bitmap-extensions
mask3-unline-find-next-bit-ia64
mask4-new-cpumask-h
mask5-remove-old-cpumask-files
mask6-cpumask-i386-fixup
mask7-cpumask-etc-fixup
mask8-rm-old-cpumask-emul
mask9-post-cleanup-tweaks
mask10-nuke-nonsmp-cpumask-ifdef

Patch name: mask10-nuke-nonsmp-cpumask-ifdef

Index: 2.6.6-rc3-mm2-bitmapv5/include/linux/cpumask.h
===================================================================
--- 2.6.6-rc3-mm2-bitmapv5.orig/include/linux/cpumask.h 2004-05-07 23:57:03.000000000 -0700
+++ 2.6.6-rc3-mm2-bitmapv5/include/linux/cpumask.h 2004-05-08 01:11:27.000000000 -0700
@@ -266,22 +266,58 @@
return bitmap_parse(buf, len, srcp->bits, nbits);
}

+#if NR_CPUS > 1
+#define for_each_cpu_mask(cpu, mask) \
+ for (cpu = first_cpu(mask); \
+ cpu < NR_CPUS; \
+ cpu = next_cpu(cpu, (mask)))
+#else /* NR_CPUS == 1 */
+#define for_each_cpu_mask(cpu, mask) for (cpu = 0; cpu < 1; cpu++)
+#endif /* NR_CPUS */
+
/*
- * The following particular system cpumasks and operations
- * on them manage all (possible) and online cpus.
+ * The following particular system cpumasks and operations manage
+ * possible, present and online cpus.
+ *
+ * Subtleties:
+ * 1) UP arch's (NR_CPUS == 1, CONFIG_SMP not defined) hardcode
+ * assumption that their single CPU is online. There are no
+ * actual cpu_{online,possible,present}_maps on UP.
+ * 2) Most SMP arch's #define some of these maps to be some
+ * other map specific to that arch.
+ * 3) Due to (2), following must be #define macros, not inlines.
+ * To see why, examine assembly code produced by following,
+ * noting that set1() writes phys_x_map (good), but set2()
+ * writes x_map (bad):
+ * int x_map, phys_x_map;
+ * #define set1(a) x_map = a
+ * inline void set2(int a) { x_map = a; }
+ * #define x_map phys_x_map
+ * main(){ set1(3); set2(5); }
+ * 4) The cpu_{online,possible,present}_maps are available as
+ * as actual defined variables even on UP arch's, though
+ * due to (1), changing them will have no useful affect on
+ * the following num_*_cpus(), cpu_*() and cpu_set_*line()
+ * macros in the UP case.
*/

-extern cpumask_t cpu_online_map;
extern cpumask_t cpu_possible_map;
+extern cpumask_t cpu_online_map;
+extern cpumask_t cpu_present_map;
+
+/* Really only do call for SMP, short circuit UP */
+#define __SMP_ONLY__(x) (NR_CPUS == 1 ? 1 : (x))

-#ifdef CONFIG_SMP
+#define num_online_cpus() __SMP_ONLY__(cpus_weight(cpu_online_map))
+#define num_possible_cpus() __SMP_ONLY__(cpus_weight(cpu_possible_map))
+#define num_present_cpus() __SMP_ONLY__(cpus_weight(cpu_present_map))
+
+#define cpu_online(cpu) __SMP_ONLY__(cpu_isset((cpu), cpu_online_map))
+#define cpu_possible(cpu) __SMP_ONLY__(cpu_isset((cpu), cpu_possible_map))
+#define cpu_present(cpu) __SMP_ONLY__(cpu_isset((cpu), cpu_present_map))

-#define num_online_cpus() cpus_weight(cpu_online_map)
-#define num_possible_cpus() cpus_weight(cpu_possible_map)
-#define cpu_online(cpu) cpu_isset((cpu), cpu_online_map)
-#define cpu_possible(cpu) cpu_isset((cpu), cpu_possible_map)
-#define cpu_set_online(cpu) cpu_set((cpu), cpu_online_map)
-#define cpu_set_offline(cpu) cpu_clear((cpu), cpu_online_map)
+#define cpu_set_online(cpu) __SMP_ONLY__(cpu_set((cpu), cpu_online_map))
+#define cpu_set_offline(cpu) __SMP_ONLY__(cpu_clear((cpu), cpu_online_map))

#define any_online_cpu(mask) \
({ \
@@ -290,34 +326,8 @@
first_cpu(m); \
})

-#define for_each_cpu_mask(cpu, mask) \
- for (cpu = first_cpu(mask); \
- cpu < NR_CPUS; \
- cpu = next_cpu(cpu, mask))
-
-#else /* !CONFIG_SMP */
-
-#define num_online_cpus() 1
-#define num_possible_cpus() 1
-#define cpu_online(cpu) ({ BUG_ON((cpu) != 0); 1; })
-#define cpu_possible(cpu) ({ BUG_ON((cpu) != 0); 1; })
-#define cpu_set_online(cpu) ({ BUG_ON((cpu) != 0); })
-#define cpu_set_offline(cpu) ({ BUG(); })
-
-#define any_online_cpu(mask) 0
-
-#define for_each_cpu_mask(cpu, mask) for (cpu = 0; cpu < 1; cpu++)
-
-#endif /* CONFIG_SMP */
-
-#define for_each_cpu(cpu) \
- for_each_cpu_mask(cpu, cpu_possible_map)
-#define for_each_online_cpu(cpu) \
- for_each_cpu_mask(cpu, cpu_online_map)
-
-extern cpumask_t cpu_present_map;
-#define num_present_cpus() cpus_weight(cpu_present_map)
-#define cpu_present(cpu) cpu_isset(cpu, cpu_present_map)
-#define for_each_present_cpu(cpu) for_each_cpu_mask(cpu, cpu_present_map)
+#define for_each_cpu(cpu) for_each_cpu_mask((cpu), cpu_possible_map)
+#define for_each_online_cpu(cpu) for_each_cpu_mask((cpu), cpu_online_map)
+#define for_each_present_cpu(cpu) for_each_cpu_mask((cpu), cpu_present_map)

#endif /* __LINUX_CPUMASK_H */
Index: 2.6.6-rc3-mm2-bitmapv5/kernel/sched.c
===================================================================
--- 2.6.6-rc3-mm2-bitmapv5.orig/kernel/sched.c 2004-05-07 23:57:03.000000000 -0700
+++ 2.6.6-rc3-mm2-bitmapv5/kernel/sched.c 2004-05-08 00:18:01.000000000 -0700
@@ -3052,16 +3052,6 @@
return retval;
}

-/*
- * Represents all cpu's present in the system
- * In systems capable of hotplug, this map could dynamically grow
- * as new cpu's are detected in the system via any platform specific
- * method, such as ACPI for e.g.
- */
-
-cpumask_t cpu_present_map;
-EXPORT_SYMBOL(cpu_present_map);
-
/**
* sys_sched_setaffinity - set the cpu affinity of a process
* @pid: pid of the process
@@ -3112,9 +3102,21 @@
return retval;
}

+/*
+ * Represents all cpu's present in the system
+ * In systems capable of hotplug, this map could dynamically grow
+ * as new cpu's are detected in the system via any platform specific
+ * method, such as ACPI for e.g.
+ */
+
+cpumask_t cpu_present_map;
+EXPORT_SYMBOL(cpu_present_map);
+
#ifndef CONFIG_SMP
cpumask_t cpu_online_map = CPU_MASK_ALL;
+EXPORT_SYMBOL(cpu_online_map);
cpumask_t cpu_possible_map = CPU_MASK_ALL;
+EXPORT_SYMBOL(cpu_possible_map);
#endif

/**

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@xxxxxxx> 1.650.933.1373
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Andrew Morton: "Re: how long does it take to init the scheduler?"
Previous message: Olaf Hering: "Re: how long does it take to init the scheduler?"
Next in thread: Paul Jackson: "Re: [PATCH mask 6/15] nonsmp-cpu-present-map"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]