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

From: Dave Hansen
Date: Mon May 21 2012 - 16:30:46 EST



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/