Hi,
Andreas Herrmann <andreas.herrmann3@amd.com> writes:
Okay, and the real and effective problem is that the code right now
aligns the relative offsets to the globally requested alignment, which
is of course total crap to do. Because, whether or not the result will
be correct (only if the node start is aligned), the global alignment is
just not applicable to relative offsets, even if it works by accident.
You are right, this was not the problem at all.
Yep.
I can not follow this example. Is there a typo somewhere? In this
example, the resulting address is aligned but it shouldn't even be so.
The sidx will be aligned and when you add 0x130000 to it, the result is
not (in this case).
All in favor for the latter. Keep the relative value at a local
alignment so that it yields a correct global alignment when combined
with node_min_pfn, no matter how node_min_pfn is aligned itself.
Attached is a version that should be easier to read. Please
double-check.
It keeps the indexes/offsets aligned relative to the node so that
combined with the node start always yields a value that is aligned as
requested.
Hannes
commit ec5b91737d0be242a6a9b255fa1749962f978188
Author: Johannes Weiner <hannes@saeurebad.de>
Date: Wed Aug 13 19:59:43 2008 +0200
bootmem: fix aligning of node-relative indexes and offsets
diff --git a/mm/bootmem.c b/mm/bootmem.c
index 4af15d0..8620832 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -405,6 +405,30 @@ int __init reserve_bootmem(unsigned long addr, unsigned long size,
}
#endif /* !CONFIG_HAVE_ARCH_BOOTMEM_NODE */
+static unsigned long align_idx(struct bootmem_data *bdata, unsigned long idx,
+ unsigned long step)
+{
+ unsigned long base = bdata->node_min_pfn;
+
+ /*
+ * Align the index with respect to the node start so that the
+ * resulting absolute PFN (node-start + index) is properly
+ * aligned.
+ */
+
+ return ALIGN(base + idx, step) - base;
+}
+
+static unsigned long align_off(struct bootmem_data *bdata, unsigned long off,
+ unsigned long align)
+{
+ unsigned long base = PFN_PHYS(bdata->node_min_pfn);
+
+ /* Same as align_idx for byte offsets */
+
+ return ALIGN(base + off, align) - base;
+}
+
static void * __init alloc_bootmem_core(struct bootmem_data *bdata,
unsigned long size, unsigned long align,
unsigned long goal, unsigned long limit)
@@ -441,16 +465,23 @@ static void * __init alloc_bootmem_core(struct bootmem_data *bdata,
else
start = ALIGN(min, step);
- sidx = start - bdata->node_min_pfn;;
+ sidx = start - bdata->node_min_pfn;
midx = max - bdata->node_min_pfn;
+ /*
+ * Because the indexes are relative to the node, all alignment
+ * below has to be done with respect to the node's start in
+ * order to have the resulting PFNs and addresses properly
+ * aligned.
+ */
+
if (bdata->hint_idx > sidx) {
/*
* Handle the valid case of sidx being zero and still
* catch the fallback below.
*/
fallback = sidx + 1;
- sidx = ALIGN(bdata->hint_idx, step);
+ sidx = align_idx(bdata, bdata->hint_idx, step);
}
while (1) {
@@ -459,7 +490,7 @@ static void * __init alloc_bootmem_core(struct bootmem_data *bdata,
unsigned long eidx, i, start_off, end_off;
find_block:
sidx = find_next_zero_bit(bdata->node_bootmem_map, midx, sidx);
- sidx = ALIGN(sidx, step);
+ sidx = align_idx(bdata, sidx, step);
eidx = sidx + PFN_UP(size);
if (sidx >= midx || eidx > midx)
@@ -467,7 +498,7 @@ find_block:
for (i = sidx; i < eidx; i++)
if (test_bit(i, bdata->node_bootmem_map)) {
- sidx = ALIGN(i, step);
+ sidx = align_idx(bdata, i, step);
if (sidx == i)
sidx += step;
goto find_block;
@@ -475,7 +506,7 @@ find_block:
if (bdata->last_end_off &&
PFN_DOWN(bdata->last_end_off) + 1 == sidx)
- start_off = ALIGN(bdata->last_end_off, align);
+ start_off = align_off(bdata, bdata->last_end_off, align);
else
start_off = PFN_PHYS(sidx);
@@ -499,7 +530,7 @@ find_block:
}
if (fallback) {
- sidx = ALIGN(fallback - 1, step);
+ sidx = align_idx(bdata, fallback - 1, step);
fallback = 0;
goto find_block;
}
--
