| Message ID | 10a3e6109c34c21a8dd4c513cf63df63481a2b07.1705085543.git.alison.schofield@intel.com |
|---|---|
| State | Accepted |
| Commit | 94a571344df867011b60e7c1399dea29410d7bd4 |
| Headers | show |
| Series | x86/numa: Fix NUMA node overlap & init failure | expand |
On Fri, Jan 12, 2024 at 12:09:50PM -0800, alison.schofield@intel.com wrote: > From: Alison Schofield <alison.schofield@intel.com> > > numa_fill_memblks() fills in the gaps in numa_meminfo memblks over a > physical address range. To do so, it first creates a list of existing > memblks that overlap that address range. The issue is that it is off > by one when comparing to the end of the address range, so memblks > that do not overlap are selected. > > The impact of selecting a memblk that does not actually overlap is > that an existing memblk may be filled when the expected action is to > do nothing and return NUMA_NO_MEMBLK to the caller. The caller can > then add a new NUMA node and memblk. > > Replace the broken open-coded search for address overlap with the > memblock helper memblock_addrs_overlap(). Update the kernel doc > and in code comments. > > Fixes: 8f012db27c95 ("x86/numa: Introduce numa_fill_memblks()") > Suggested by: "Huang, Ying" <ying.huang@intel.com> > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > --- > > Changes since single patch: > - Use memblock_addrs_overlap() for address comparison (Dan) > - Update kernel doc and in code comments > - New commit message > - Update commit log > - Link to single patch: > https://lore.kernel.org/linux-cxl/20240102213206.1493733-1-alison.schofield@intel.com/ > > arch/x86/mm/numa.c | 19 +++++++------------ > include/linux/memblock.h | 2 ++ > mm/memblock.c | 5 +++-- For memblock changes Acked-by: Mike Rapoport (IBM) <rppt@kernel.org> > 3 files changed, 12 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c > index adc497b93f03..8ada9bbfad58 100644 > --- a/arch/x86/mm/numa.c > +++ b/arch/x86/mm/numa.c > @@ -944,14 +944,12 @@ static struct numa_memblk *numa_memblk_list[NR_NODE_MEMBLKS] __initdata; > * @start: address to begin fill > * @end: address to end fill > * > - * Find and extend numa_meminfo memblks to cover the @start-@end > - * physical address range, such that the first memblk includes > - * @start, the last memblk includes @end, and any gaps in between > - * are filled. > + * Find and extend numa_meminfo memblks to cover the physical > + * address range @start-@end > * > * RETURNS: > * 0 : Success > - * NUMA_NO_MEMBLK : No memblk exists in @start-@end range > + * NUMA_NO_MEMBLK : No memblks exist in address range @start-@end > */ > > int __init numa_fill_memblks(u64 start, u64 end) > @@ -963,17 +961,14 @@ int __init numa_fill_memblks(u64 start, u64 end) > > /* > * Create a list of pointers to numa_meminfo memblks that > - * overlap start, end. Exclude (start == bi->end) since > - * end addresses in both a CFMWS range and a memblk range > - * are exclusive. > - * > - * This list of pointers is used to make in-place changes > - * that fill out the numa_meminfo memblks. > + * overlap start, end. The list is used to make in-place > + * changes that fill out the numa_meminfo memblks. > */ > for (int i = 0; i < mi->nr_blks; i++) { > struct numa_memblk *bi = &mi->blk[i]; > > - if (start < bi->end && end >= bi->start) { > + if (memblock_addrs_overlap(start, end - start, bi->start, > + bi->end - bi->start)) { > blk[count] = &mi->blk[i]; > count++; > } > diff --git a/include/linux/memblock.h b/include/linux/memblock.h > index b695f9e946da..e2082240586d 100644 > --- a/include/linux/memblock.h > +++ b/include/linux/memblock.h > @@ -121,6 +121,8 @@ int memblock_reserve(phys_addr_t base, phys_addr_t size); > int memblock_physmem_add(phys_addr_t base, phys_addr_t size); > #endif > void memblock_trim_memory(phys_addr_t align); > +unsigned long memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1, > + phys_addr_t base2, phys_addr_t size2); > bool memblock_overlaps_region(struct memblock_type *type, > phys_addr_t base, phys_addr_t size); > bool memblock_validate_numa_coverage(unsigned long threshold_bytes); > diff --git a/mm/memblock.c b/mm/memblock.c > index 8c194d8afeec..d0cadeeecfe8 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -180,8 +180,9 @@ static inline phys_addr_t memblock_cap_size(phys_addr_t base, phys_addr_t *size) > /* > * Address comparison utilities > */ > -static unsigned long __init_memblock memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1, > - phys_addr_t base2, phys_addr_t size2) > +unsigned long __init_memblock > +memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1, phys_addr_t base2, > + phys_addr_t size2) > { > return ((base1 < (base2 + size2)) && (base2 < (base1 + size1))); > } > -- > 2.37.3 >
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c index adc497b93f03..8ada9bbfad58 100644 --- a/arch/x86/mm/numa.c +++ b/arch/x86/mm/numa.c @@ -944,14 +944,12 @@ static struct numa_memblk *numa_memblk_list[NR_NODE_MEMBLKS] __initdata; * @start: address to begin fill * @end: address to end fill * - * Find and extend numa_meminfo memblks to cover the @start-@end - * physical address range, such that the first memblk includes - * @start, the last memblk includes @end, and any gaps in between - * are filled. + * Find and extend numa_meminfo memblks to cover the physical + * address range @start-@end * * RETURNS: * 0 : Success - * NUMA_NO_MEMBLK : No memblk exists in @start-@end range + * NUMA_NO_MEMBLK : No memblks exist in address range @start-@end */ int __init numa_fill_memblks(u64 start, u64 end) @@ -963,17 +961,14 @@ int __init numa_fill_memblks(u64 start, u64 end) /* * Create a list of pointers to numa_meminfo memblks that - * overlap start, end. Exclude (start == bi->end) since - * end addresses in both a CFMWS range and a memblk range - * are exclusive. - * - * This list of pointers is used to make in-place changes - * that fill out the numa_meminfo memblks. + * overlap start, end. The list is used to make in-place + * changes that fill out the numa_meminfo memblks. */ for (int i = 0; i < mi->nr_blks; i++) { struct numa_memblk *bi = &mi->blk[i]; - if (start < bi->end && end >= bi->start) { + if (memblock_addrs_overlap(start, end - start, bi->start, + bi->end - bi->start)) { blk[count] = &mi->blk[i]; count++; } diff --git a/include/linux/memblock.h b/include/linux/memblock.h index b695f9e946da..e2082240586d 100644 --- a/include/linux/memblock.h +++ b/include/linux/memblock.h @@ -121,6 +121,8 @@ int memblock_reserve(phys_addr_t base, phys_addr_t size); int memblock_physmem_add(phys_addr_t base, phys_addr_t size); #endif void memblock_trim_memory(phys_addr_t align); +unsigned long memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1, + phys_addr_t base2, phys_addr_t size2); bool memblock_overlaps_region(struct memblock_type *type, phys_addr_t base, phys_addr_t size); bool memblock_validate_numa_coverage(unsigned long threshold_bytes); diff --git a/mm/memblock.c b/mm/memblock.c index 8c194d8afeec..d0cadeeecfe8 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -180,8 +180,9 @@ static inline phys_addr_t memblock_cap_size(phys_addr_t base, phys_addr_t *size) /* * Address comparison utilities */ -static unsigned long __init_memblock memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1, - phys_addr_t base2, phys_addr_t size2) +unsigned long __init_memblock +memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1, phys_addr_t base2, + phys_addr_t size2) { return ((base1 < (base2 + size2)) && (base2 < (base1 + size1))); }