[RFC][PATCH 2/2] sparsemem: fix boot when SECTIONS_PER_ROOT is not power-of-2

[Dave Hansen]

I was getting some interesting oopses at boot after adding a
field to 'struct mem_section'.  I tracked it down to
__nr_to_section().  In my case, SECTIONS_PER_ROOT got set to
73, which leads to an interesting bitmask:

	#define SECTIONS_PER_ROOT       (PAGE_SIZE / sizeof (struct mem_section))
	...
	#define SECTION_ROOT_MASK      (SECTIONS_PER_ROOT - 1)

We only use SECTION_ROOT_MASK in one place.  If we replace it
with some modulo arithmetic, it compiles down to the same thing
for power-of-2 SECTIONS_PER_ROOT values, but it also actually
*works* instead of just failing to boot at some random point
for the other case.

This also adds some requisite comments in the structure for
future hapless kernel developers, plus a bonus WARN_ON_ONCE()
just in case they miss the big fat comment.

Granted, this patch is not fixing a bug that anyone will really
hit in practice, but it will surely save future developers a
headache or two, plus it removes a #define!

Signed-off-by: Dave Hansen <dave@xxxxxxxxxxxxxxxxxx>
---

 linux-2.6.git-dave/include/linux/mmzone.h |    9 +++++++--
 linux-2.6.git-dave/mm/sparse.c            |    5 +++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff -puN include/linux/mmzone.h~sparsemem-fix-boot-when-SECTIONS_PER_ROOT-is-not-power_of_2 include/linux/mmzone.h
--- linux-2.6.git/include/linux/mmzone.h~sparsemem-fix-boot-when-SECTIONS_PER_ROOT-is-not-power_of_2	2012-05-21 13:29:43.777274223 -0700
+++ linux-2.6.git-dave/include/linux/mmzone.h	2012-05-21 13:29:43.789274356 -0700
@@ -1018,6 +1018,12 @@ struct mem_section {
 	struct page_cgroup *page_cgroup;
 	unsigned long pad;
 #endif
+	/*
+	 * WARNING: Do not put any fields here that could cause
+	 * this structure to become a non-power-of-2 size.
+	 * Operations like pfn_to_page() will end up doing
+	 * division in hot paths for CONFIG_SPARSEMEM_EXTREME.
+	 */
 };
 
 #ifdef CONFIG_SPARSEMEM_EXTREME
@@ -1028,7 +1034,6 @@ struct mem_section {
 
 #define SECTION_NR_TO_ROOT(sec)	((sec) / SECTIONS_PER_ROOT)
 #define NR_SECTION_ROOTS	DIV_ROUND_UP(NR_MEM_SECTIONS, SECTIONS_PER_ROOT)
-#define SECTION_ROOT_MASK	(SECTIONS_PER_ROOT - 1)
 
 #ifdef CONFIG_SPARSEMEM_EXTREME
 extern struct mem_section *mem_section[NR_SECTION_ROOTS];
@@ -1040,7 +1045,7 @@ static inline struct mem_section *__nr_t
 {
 	if (!mem_section[SECTION_NR_TO_ROOT(nr)])
 		return NULL;
-	return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK];
+	return &mem_section[SECTION_NR_TO_ROOT(nr)][nr % SECTIONS_PER_ROOT];
 }
 extern int __section_nr(struct mem_section* ms);
 extern unsigned long usemap_size(void);
diff -puN mm/sparse.c~sparsemem-fix-boot-when-SECTIONS_PER_ROOT-is-not-power_of_2 mm/sparse.c
--- linux-2.6.git/mm/sparse.c~sparsemem-fix-boot-when-SECTIONS_PER_ROOT-is-not-power_of_2	2012-05-21 13:29:43.781274268 -0700
+++ linux-2.6.git-dave/mm/sparse.c	2012-05-21 13:29:43.789274356 -0700
@@ -63,6 +63,11 @@ static struct mem_section noinline __ini
 	unsigned long array_size = SECTIONS_PER_ROOT *
 				   sizeof(struct mem_section);
 
+	/*
+	 * See note in 'struct mem_section' definition
+	 */
+	WARN_ON_ONCE(!is_power_of_2(sizeof(struct mem_section)));
+
 	if (slab_is_available()) {
 		if (node_state(nid, N_HIGH_MEMORY))
 			section = kmalloc_node(array_size, GFP_KERNEL, nid);
_

--
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/