Changes since v8 [1]:
- Rebase on next-20190604 to incorporate the removal of the
MHP_MEMBLOCK_API flag and other cleanups from David.
- Move definition of subsection_mask_set() earlier into "mm/sparsemem:
Add helpers track active portions of a section at boot" (Oscar)
- Cleanup unnecessary IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP) in
section_deactivate() in response to a request (declined) to split the
pure CONFIG_SPARSEMEM bits from section_{de,}activate(). I submit that
the maintenance is less error prone, especially when modifying common
logic, if the implementations remain unified. (Oscar)
- Cleanup sparse_add_section() vs sparse_index_init() return code.
(Oscar)
- Document ZONE_DEVICE and subsection semantics relative to
CONFIG_SPARSEMEM_VMEMMAP in Documentation/vm/memory-model.rst. (Mike)
[1]: https://lore.kernel.org/lkml/155718596657.130019.17139634728875079809.stgit@dwillia2-desk3.amr.corp.intel.com/
---
The memory hotplug section is an arbitrary / convenient unit for memory
hotplug. 'Section-size' units have bled into the user interface
('memblock' sysfs) and can not be changed without breaking existing
userspace. The section-size constraint, while mostly benign for typical
memory hotplug, has and continues to wreak havoc with 'device-memory'
use cases, persistent memory (pmem) in particular. Recall that pmem uses
devm_memremap_pages(), and subsequently arch_add_memory(), to allocate a
'struct page' memmap for pmem. However, it does not use the 'bottom
half' of memory hotplug, i.e. never marks pmem pages online and never
exposes the userspace memblock interface for pmem. This leaves an
opening to redress the section-size constraint.
To date, the libnvdimm subsystem has attempted to inject padding to
satisfy the internal constraints of arch_add_memory(). Beyond
complicating the code, leading to bugs [2], wasting memory, and limiting
configuration flexibility, the padding hack is broken when the platform
changes this physical memory alignment of pmem from one boot to the
next. Device failure (intermittent or permanent) and physical
reconfiguration are events that can cause the platform firmware to
change the physical placement of pmem on a subsequent boot, and device
failure is an everyday event in a data-center.
It turns out that sections are only a hard requirement of the
user-facing interface for memory hotplug and with a bit more
infrastructure sub-section arch_add_memory() support can be added for
kernel internal usages like devm_memremap_pages(). Here is an analysis
of the current design assumptions in the current code and how they are
addressed in the new implementation:
Current design assumptions:
- Sections that describe boot memory (early sections) are never
unplugged / removed.
- pfn_valid(), in the CONFIG_SPARSEMEM_VMEMMAP=y, case devolves to a
valid_section() check
- __add_pages() and helper routines assume all operations occur in
PAGES_PER_SECTION units.
- The memblock sysfs interface only comprehends full sections
New design assumptions:
- Sections are instrumented with a sub-section bitmask to track (on x86)
individual 2MB sub-divisions of a 128MB section.
- Partially populated early sections can be extended with additional
sub-sections, and those sub-sections can be removed with
arch_remove_memory(). With this in place we no longer lose usable memory
capacity to padding.
- pfn_valid() is updated to look deeper than valid_section() to also check the
active-sub-section mask. This indication is in the same cacheline as
the valid_section() so the performance impact is expected to be
negligible. So far the lkp robot has not reported any regressions.
- Outside of the core vmemmap population routines which are replaced,
other helper routines like shrink_{zone,pgdat}_span() are updated to
handle the smaller granularity. Core memory hotplug routines that deal
with online memory are not touched.
- The existing memblock sysfs user api guarantees / assumptions are
not touched since this capability is limited to !online
!memblock-sysfs-accessible sections.
Meanwhile the issue reports continue to roll in from users that do not
understand when and how the 128MB constraint will bite them. The current
implementation relied on being able to support at least one misaligned
namespace, but that immediately falls over on any moderately complex
namespace creation attempt. Beyond the initial problem of 'System RAM'
colliding with pmem, and the unsolvable problem of physical alignment
changes, Linux is now being exposed to platforms that collide pmem
ranges with other pmem ranges by default [3]. In short,
devm_memremap_pages() has pushed the venerable section-size constraint
past the breaking point, and the simplicity of section-aligned
arch_add_memory() is no longer tenable.
These patches are exposed to the kbuild robot on my libnvdimm-pending
branch [4], and a preview of the unit test for this functionality is
available on the 'subsection-pending' branch of ndctl [5].
[2]: https://lore.kernel.org/r/155000671719.348031.2347363160141119237.stgit@dwillia2-desk3.amr.corp.intel.com
[3]: https://github.com/pmem/ndctl/issues/76
[4]: https://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm.git/log/?h=libnvdimm-pending
[5]: https://github.com/pmem/ndctl/commit/7c59b4867e1c
---
Dan Williams (12):
mm/sparsemem: Introduce struct mem_section_usage
mm/sparsemem: Add helpers track active portions of a section at boot
mm/hotplug: Prepare shrink_{zone,pgdat}_span for sub-section removal
mm/sparsemem: Convert kmalloc_section_memmap() to populate_section_memmap()
mm/hotplug: Kill is_dev_zone() usage in __remove_pages()
mm: Kill is_dev_zone() helper
mm/sparsemem: Prepare for sub-section ranges
mm/sparsemem: Support sub-section hotplug
mm: Document ZONE_DEVICE memory-model implications
mm/devm_memremap_pages: Enable sub-section remap
libnvdimm/pfn: Fix fsdax-mode namespace info-block zero-fields
libnvdimm/pfn: Stop padding pmem namespaces to section alignment
Documentation/vm/memory-model.rst | 39 ++++
arch/powerpc/include/asm/sparsemem.h | 3
arch/x86/mm/init_64.c | 4
drivers/nvdimm/dax_devs.c | 2
drivers/nvdimm/pfn.h | 15 -
drivers/nvdimm/pfn_devs.c | 95 +++------
include/linux/memory_hotplug.h | 7 -
include/linux/mm.h | 4
include/linux/mmzone.h | 92 +++++++--
kernel/memremap.c | 61 ++----
mm/memory_hotplug.c | 171 +++++++++-------
mm/page_alloc.c | 10 +
mm/sparse-vmemmap.c | 21 +-
mm/sparse.c | 359 +++++++++++++++++++++++-----------
14 files changed, 534 insertions(+), 349 deletions(-)
Prepare for hot{plug,remove} of sub-ranges of a section by tracking a
sub-section active bitmask, each bit representing a PMD_SIZE span of the
architecture's memory hotplug section size.
The implications of a partially populated section is that pfn_valid()
needs to go beyond a valid_section() check and read the sub-section
active ranges from the bitmask. The expectation is that the bitmask
(subsection_map) fits in the same cacheline as the valid_section() data,
so the incremental performance overhead to pfn_valid() should be
negligible.
Cc: Michal Hocko <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Pavel Tatashin <[email protected]>
Tested-by: Jane Chu <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
include/linux/mmzone.h | 29 ++++++++++++++++++++++++++++-
mm/page_alloc.c | 4 +++-
mm/sparse.c | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index ac163f2f274f..6dd52d544857 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1199,6 +1199,8 @@ struct mem_section_usage {
unsigned long pageblock_flags[0];
};
+void subsection_map_init(unsigned long pfn, unsigned long nr_pages);
+
struct page;
struct page_ext;
struct mem_section {
@@ -1336,12 +1338,36 @@ static inline struct mem_section *__pfn_to_section(unsigned long pfn)
extern int __highest_present_section_nr;
+static inline int subsection_map_index(unsigned long pfn)
+{
+ return (pfn & ~(PAGE_SECTION_MASK)) / PAGES_PER_SUBSECTION;
+}
+
+#ifdef CONFIG_SPARSEMEM_VMEMMAP
+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
+{
+ int idx = subsection_map_index(pfn);
+
+ return test_bit(idx, ms->usage->subsection_map);
+}
+#else
+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
+{
+ return 1;
+}
+#endif
+
#ifndef CONFIG_HAVE_ARCH_PFN_VALID
static inline int pfn_valid(unsigned long pfn)
{
+ struct mem_section *ms;
+
if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
return 0;
- return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
+ ms = __nr_to_section(pfn_to_section_nr(pfn));
+ if (!valid_section(ms))
+ return 0;
+ return pfn_section_valid(ms, pfn);
}
#endif
@@ -1373,6 +1399,7 @@ void sparse_init(void);
#define sparse_init() do {} while (0)
#define sparse_index_init(_sec, _nid) do {} while (0)
#define pfn_present pfn_valid
+#define subsection_map_init(_pfn, _nr_pages) do {} while (0)
#endif /* CONFIG_SPARSEMEM */
/*
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c6d8224d792e..bd773efe5b82 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7292,10 +7292,12 @@ void __init free_area_init_nodes(unsigned long *max_zone_pfn)
/* Print out the early node map */
pr_info("Early memory node ranges\n");
- for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid)
+ for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid) {
pr_info(" node %3d: [mem %#018Lx-%#018Lx]\n", nid,
(u64)start_pfn << PAGE_SHIFT,
((u64)end_pfn << PAGE_SHIFT) - 1);
+ subsection_map_init(start_pfn, end_pfn - start_pfn);
+ }
/* Initialise every node */
mminit_verify_pageflags_layout();
diff --git a/mm/sparse.c b/mm/sparse.c
index 71da15cc7432..0baa2e55cfdd 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -210,6 +210,41 @@ static inline unsigned long first_present_section_nr(void)
return next_present_section_nr(-1);
}
+void subsection_mask_set(unsigned long *map, unsigned long pfn,
+ unsigned long nr_pages)
+{
+ int idx = subsection_map_index(pfn);
+ int end = subsection_map_index(pfn + nr_pages - 1);
+
+ bitmap_set(map, idx, end - idx + 1);
+}
+
+void subsection_map_init(unsigned long pfn, unsigned long nr_pages)
+{
+ int end_sec = pfn_to_section_nr(pfn + nr_pages - 1);
+ int i, start_sec = pfn_to_section_nr(pfn);
+
+ if (!nr_pages)
+ return;
+
+ for (i = start_sec; i <= end_sec; i++) {
+ struct mem_section *ms;
+ unsigned long pfns;
+
+ pfns = min(nr_pages, PAGES_PER_SECTION
+ - (pfn & ~PAGE_SECTION_MASK));
+ ms = __nr_to_section(i);
+ subsection_mask_set(ms->usage->subsection_map, pfn, pfns);
+
+ pr_debug("%s: sec: %d pfns: %ld set(%d, %d)\n", __func__, i,
+ pfns, subsection_map_index(pfn),
+ subsection_map_index(pfn + pfns - 1));
+
+ pfn += pfns;
+ nr_pages -= pfns;
+ }
+}
+
/* Record a memory area against a node. */
void __init memory_present(int nid, unsigned long start, unsigned long end)
{
Towards enabling memory hotplug to track partial population of a
section, introduce 'struct mem_section_usage'.
A pointer to a 'struct mem_section_usage' instance replaces the existing
pointer to a 'pageblock_flags' bitmap. Effectively it adds one more
'unsigned long' beyond the 'pageblock_flags' (usemap) allocation to
house a new 'subsection_map' bitmap. The new bitmap enables the memory
hot{plug,remove} implementation to act on incremental sub-divisions of a
section.
The default SUBSECTION_SHIFT is chosen to keep the 'subsection_map' no
larger than a single 'unsigned long' on the major architectures.
Alternatively an architecture can define ARCH_SUBSECTION_SHIFT to
override the default PMD_SHIFT. Note that PowerPC needs to use
ARCH_SUBSECTION_SHIFT to workaround PMD_SHIFT being a non-constant
expression on PowerPC.
The primary motivation for this functionality is to support platforms
that mix "System RAM" and "Persistent Memory" within a single section,
or multiple PMEM ranges with different mapping lifetimes within a single
section. The section restriction for hotplug has caused an ongoing saga
of hacks and bugs for devm_memremap_pages() users.
Beyond the fixups to teach existing paths how to retrieve the 'usemap'
from a section, and updates to usemap allocation path, there are no
expected behavior changes.
Cc: Michal Hocko <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Pavel Tatashin <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
arch/powerpc/include/asm/sparsemem.h | 3 +
include/linux/mmzone.h | 48 +++++++++++++++++++-
mm/memory_hotplug.c | 18 ++++----
mm/page_alloc.c | 2 -
mm/sparse.c | 81 +++++++++++++++++-----------------
5 files changed, 99 insertions(+), 53 deletions(-)
diff --git a/arch/powerpc/include/asm/sparsemem.h b/arch/powerpc/include/asm/sparsemem.h
index 3192d454a733..1aa3c9303bf8 100644
--- a/arch/powerpc/include/asm/sparsemem.h
+++ b/arch/powerpc/include/asm/sparsemem.h
@@ -10,6 +10,9 @@
*/
#define SECTION_SIZE_BITS 24
+/* Reflect the largest possible PMD-size as the subsection-size constant */
+#define ARCH_SUBSECTION_SHIFT 24
+
#endif /* CONFIG_SPARSEMEM */
#ifdef CONFIG_MEMORY_HOTPLUG
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 427b79c39b3c..ac163f2f274f 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1161,6 +1161,44 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
#define SECTION_ALIGN_UP(pfn) (((pfn) + PAGES_PER_SECTION - 1) & PAGE_SECTION_MASK)
#define SECTION_ALIGN_DOWN(pfn) ((pfn) & PAGE_SECTION_MASK)
+/*
+ * SUBSECTION_SHIFT must be constant since it is used to declare
+ * subsection_map and related bitmaps without triggering the generation
+ * of variable-length arrays. The most natural size for a subsection is
+ * a PMD-page. For architectures that do not have a constant PMD-size
+ * ARCH_SUBSECTION_SHIFT can be set to a constant max size, or otherwise
+ * fallback to 2MB.
+ */
+#if defined(ARCH_SUBSECTION_SHIFT)
+#define SUBSECTION_SHIFT (ARCH_SUBSECTION_SHIFT)
+#elif defined(PMD_SHIFT)
+#define SUBSECTION_SHIFT (PMD_SHIFT)
+#else
+/*
+ * Memory hotplug enabled platforms avoid this default because they
+ * either define ARCH_SUBSECTION_SHIFT, or PMD_SHIFT is a constant, but
+ * this is kept as a backstop to allow compilation on
+ * !ARCH_ENABLE_MEMORY_HOTPLUG archs.
+ */
+#define SUBSECTION_SHIFT 21
+#endif
+
+#define PFN_SUBSECTION_SHIFT (SUBSECTION_SHIFT - PAGE_SHIFT)
+#define PAGES_PER_SUBSECTION (1UL << PFN_SUBSECTION_SHIFT)
+#define PAGE_SUBSECTION_MASK ((~(PAGES_PER_SUBSECTION-1)))
+
+#if SUBSECTION_SHIFT > SECTION_SIZE_BITS
+#error Subsection size exceeds section size
+#else
+#define SUBSECTIONS_PER_SECTION (1UL << (SECTION_SIZE_BITS - SUBSECTION_SHIFT))
+#endif
+
+struct mem_section_usage {
+ DECLARE_BITMAP(subsection_map, SUBSECTIONS_PER_SECTION);
+ /* See declaration of similar field in struct zone */
+ unsigned long pageblock_flags[0];
+};
+
struct page;
struct page_ext;
struct mem_section {
@@ -1178,8 +1216,7 @@ struct mem_section {
*/
unsigned long section_mem_map;
- /* See declaration of similar field in struct zone */
- unsigned long *pageblock_flags;
+ struct mem_section_usage *usage;
#ifdef CONFIG_PAGE_EXTENSION
/*
* If SPARSEMEM, pgdat doesn't have page_ext pointer. We use
@@ -1210,6 +1247,11 @@ extern struct mem_section **mem_section;
extern struct mem_section mem_section[NR_SECTION_ROOTS][SECTIONS_PER_ROOT];
#endif
+static inline unsigned long *section_to_usemap(struct mem_section *ms)
+{
+ return ms->usage->pageblock_flags;
+}
+
static inline struct mem_section *__nr_to_section(unsigned long nr)
{
#ifdef CONFIG_SPARSEMEM_EXTREME
@@ -1221,7 +1263,7 @@ static inline struct mem_section *__nr_to_section(unsigned long nr)
return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK];
}
extern int __section_nr(struct mem_section* ms);
-extern unsigned long usemap_size(void);
+extern size_t mem_section_usage_size(void);
/*
* We use the lower bits of the mem_map pointer to store
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index a88c5f334e5a..7b963c2d3a0d 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -166,9 +166,10 @@ void put_page_bootmem(struct page *page)
#ifndef CONFIG_SPARSEMEM_VMEMMAP
static void register_page_bootmem_info_section(unsigned long start_pfn)
{
- unsigned long *usemap, mapsize, section_nr, i;
+ unsigned long mapsize, section_nr, i;
struct mem_section *ms;
struct page *page, *memmap;
+ struct mem_section_usage *usage;
section_nr = pfn_to_section_nr(start_pfn);
ms = __nr_to_section(section_nr);
@@ -188,10 +189,10 @@ static void register_page_bootmem_info_section(unsigned long start_pfn)
for (i = 0; i < mapsize; i++, page++)
get_page_bootmem(section_nr, page, SECTION_INFO);
- usemap = ms->pageblock_flags;
- page = virt_to_page(usemap);
+ usage = ms->usage;
+ page = virt_to_page(usage);
- mapsize = PAGE_ALIGN(usemap_size()) >> PAGE_SHIFT;
+ mapsize = PAGE_ALIGN(mem_section_usage_size()) >> PAGE_SHIFT;
for (i = 0; i < mapsize; i++, page++)
get_page_bootmem(section_nr, page, MIX_SECTION_INFO);
@@ -200,9 +201,10 @@ static void register_page_bootmem_info_section(unsigned long start_pfn)
#else /* CONFIG_SPARSEMEM_VMEMMAP */
static void register_page_bootmem_info_section(unsigned long start_pfn)
{
- unsigned long *usemap, mapsize, section_nr, i;
+ unsigned long mapsize, section_nr, i;
struct mem_section *ms;
struct page *page, *memmap;
+ struct mem_section_usage *usage;
section_nr = pfn_to_section_nr(start_pfn);
ms = __nr_to_section(section_nr);
@@ -211,10 +213,10 @@ static void register_page_bootmem_info_section(unsigned long start_pfn)
register_page_bootmem_memmap(section_nr, memmap, PAGES_PER_SECTION);
- usemap = ms->pageblock_flags;
- page = virt_to_page(usemap);
+ usage = ms->usage;
+ page = virt_to_page(usage);
- mapsize = PAGE_ALIGN(usemap_size()) >> PAGE_SHIFT;
+ mapsize = PAGE_ALIGN(mem_section_usage_size()) >> PAGE_SHIFT;
for (i = 0; i < mapsize; i++, page++)
get_page_bootmem(section_nr, page, MIX_SECTION_INFO);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c061f66c2d0c..c6d8224d792e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -404,7 +404,7 @@ static inline unsigned long *get_pageblock_bitmap(struct page *page,
unsigned long pfn)
{
#ifdef CONFIG_SPARSEMEM
- return __pfn_to_section(pfn)->pageblock_flags;
+ return section_to_usemap(__pfn_to_section(pfn));
#else
return page_zone(page)->pageblock_flags;
#endif /* CONFIG_SPARSEMEM */
diff --git a/mm/sparse.c b/mm/sparse.c
index 1552c855d62a..71da15cc7432 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -288,33 +288,31 @@ struct page *sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long pn
static void __meminit sparse_init_one_section(struct mem_section *ms,
unsigned long pnum, struct page *mem_map,
- unsigned long *pageblock_bitmap)
+ struct mem_section_usage *usage)
{
ms->section_mem_map &= ~SECTION_MAP_MASK;
ms->section_mem_map |= sparse_encode_mem_map(mem_map, pnum) |
SECTION_HAS_MEM_MAP;
- ms->pageblock_flags = pageblock_bitmap;
+ ms->usage = usage;
}
-unsigned long usemap_size(void)
+static unsigned long usemap_size(void)
{
return BITS_TO_LONGS(SECTION_BLOCKFLAGS_BITS) * sizeof(unsigned long);
}
-#ifdef CONFIG_MEMORY_HOTPLUG
-static unsigned long *__kmalloc_section_usemap(void)
+size_t mem_section_usage_size(void)
{
- return kmalloc(usemap_size(), GFP_KERNEL);
+ return sizeof(struct mem_section_usage) + usemap_size();
}
-#endif /* CONFIG_MEMORY_HOTPLUG */
#ifdef CONFIG_MEMORY_HOTREMOVE
-static unsigned long * __init
+static struct mem_section_usage * __init
sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat,
unsigned long size)
{
+ struct mem_section_usage *usage;
unsigned long goal, limit;
- unsigned long *p;
int nid;
/*
* A page may contain usemaps for other sections preventing the
@@ -330,15 +328,16 @@ sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat,
limit = goal + (1UL << PA_SECTION_SHIFT);
nid = early_pfn_to_nid(goal >> PAGE_SHIFT);
again:
- p = memblock_alloc_try_nid(size, SMP_CACHE_BYTES, goal, limit, nid);
- if (!p && limit) {
+ usage = memblock_alloc_try_nid(size, SMP_CACHE_BYTES, goal, limit, nid);
+ if (!usage && limit) {
limit = 0;
goto again;
}
- return p;
+ return usage;
}
-static void __init check_usemap_section_nr(int nid, unsigned long *usemap)
+static void __init check_usemap_section_nr(int nid,
+ struct mem_section_usage *usage)
{
unsigned long usemap_snr, pgdat_snr;
static unsigned long old_usemap_snr;
@@ -352,7 +351,7 @@ static void __init check_usemap_section_nr(int nid, unsigned long *usemap)
old_pgdat_snr = NR_MEM_SECTIONS;
}
- usemap_snr = pfn_to_section_nr(__pa(usemap) >> PAGE_SHIFT);
+ usemap_snr = pfn_to_section_nr(__pa(usage) >> PAGE_SHIFT);
pgdat_snr = pfn_to_section_nr(__pa(pgdat) >> PAGE_SHIFT);
if (usemap_snr == pgdat_snr)
return;
@@ -380,14 +379,15 @@ static void __init check_usemap_section_nr(int nid, unsigned long *usemap)
usemap_snr, pgdat_snr, nid);
}
#else
-static unsigned long * __init
+static struct mem_section_usage * __init
sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat,
unsigned long size)
{
return memblock_alloc_node(size, SMP_CACHE_BYTES, pgdat->node_id);
}
-static void __init check_usemap_section_nr(int nid, unsigned long *usemap)
+static void __init check_usemap_section_nr(int nid,
+ struct mem_section_usage *usage)
{
}
#endif /* CONFIG_MEMORY_HOTREMOVE */
@@ -474,14 +474,13 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,
unsigned long pnum_end,
unsigned long map_count)
{
- unsigned long pnum, usemap_longs, *usemap;
+ struct mem_section_usage *usage;
+ unsigned long pnum;
struct page *map;
- usemap_longs = BITS_TO_LONGS(SECTION_BLOCKFLAGS_BITS);
- usemap = sparse_early_usemaps_alloc_pgdat_section(NODE_DATA(nid),
- usemap_size() *
- map_count);
- if (!usemap) {
+ usage = sparse_early_usemaps_alloc_pgdat_section(NODE_DATA(nid),
+ mem_section_usage_size() * map_count);
+ if (!usage) {
pr_err("%s: node[%d] usemap allocation failed", __func__, nid);
goto failed;
}
@@ -497,9 +496,9 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,
pnum_begin = pnum;
goto failed;
}
- check_usemap_section_nr(nid, usemap);
- sparse_init_one_section(__nr_to_section(pnum), pnum, map, usemap);
- usemap += usemap_longs;
+ check_usemap_section_nr(nid, usage);
+ sparse_init_one_section(__nr_to_section(pnum), pnum, map, usage);
+ usage = (void *) usage + mem_section_usage_size();
}
sparse_buffer_fini();
return;
@@ -697,9 +696,9 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
struct vmem_altmap *altmap)
{
unsigned long section_nr = pfn_to_section_nr(start_pfn);
+ struct mem_section_usage *usage;
struct mem_section *ms;
struct page *memmap;
- unsigned long *usemap;
int ret;
/*
@@ -713,8 +712,8 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
memmap = kmalloc_section_memmap(section_nr, nid, altmap);
if (!memmap)
return -ENOMEM;
- usemap = __kmalloc_section_usemap();
- if (!usemap) {
+ usage = kzalloc(mem_section_usage_size(), GFP_KERNEL);
+ if (!usage) {
__kfree_section_memmap(memmap, altmap);
return -ENOMEM;
}
@@ -732,11 +731,11 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
page_init_poison(memmap, sizeof(struct page) * PAGES_PER_SECTION);
section_mark_present(ms);
- sparse_init_one_section(ms, section_nr, memmap, usemap);
+ sparse_init_one_section(ms, section_nr, memmap, usage);
out:
if (ret < 0) {
- kfree(usemap);
+ kfree(usage);
__kfree_section_memmap(memmap, altmap);
}
return ret;
@@ -772,20 +771,20 @@ static inline void clear_hwpoisoned_pages(struct page *memmap, int nr_pages)
}
#endif
-static void free_section_usemap(struct page *memmap, unsigned long *usemap,
- struct vmem_altmap *altmap)
+static void free_section_usage(struct page *memmap,
+ struct mem_section_usage *usage, struct vmem_altmap *altmap)
{
- struct page *usemap_page;
+ struct page *usage_page;
- if (!usemap)
+ if (!usage)
return;
- usemap_page = virt_to_page(usemap);
+ usage_page = virt_to_page(usage);
/*
* Check to see if allocation came from hot-plug-add
*/
- if (PageSlab(usemap_page) || PageCompound(usemap_page)) {
- kfree(usemap);
+ if (PageSlab(usage_page) || PageCompound(usage_page)) {
+ kfree(usage);
if (memmap)
__kfree_section_memmap(memmap, altmap);
return;
@@ -804,18 +803,18 @@ void sparse_remove_one_section(struct mem_section *ms, unsigned long map_offset,
struct vmem_altmap *altmap)
{
struct page *memmap = NULL;
- unsigned long *usemap = NULL;
+ struct mem_section_usage *usage = NULL;
if (ms->section_mem_map) {
- usemap = ms->pageblock_flags;
+ usage = ms->usage;
memmap = sparse_decode_mem_map(ms->section_mem_map,
__section_nr(ms));
ms->section_mem_map = 0;
- ms->pageblock_flags = NULL;
+ ms->usage = NULL;
}
clear_hwpoisoned_pages(memmap + map_offset,
PAGES_PER_SECTION - map_offset);
- free_section_usemap(memmap, usemap, altmap);
+ free_section_usage(memmap, usage, altmap);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
Allow sub-section sized ranges to be added to the memmap.
populate_section_memmap() takes an explict pfn range rather than
assuming a full section, and those parameters are plumbed all the way
through to vmmemap_populate(). There should be no sub-section usage in
current deployments. New warnings are added to clarify which memmap
allocation paths are sub-section capable.
Cc: Michal Hocko <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Cc: Oscar Salvador <[email protected]>
Reviewed-by: Pavel Tatashin <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
arch/x86/mm/init_64.c | 4 ++-
include/linux/mm.h | 4 ++-
mm/sparse-vmemmap.c | 21 +++++++++++------
mm/sparse.c | 61 +++++++++++++++++++++++++++++++------------------
4 files changed, 57 insertions(+), 33 deletions(-)
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 8335ac6e1112..688fb0687e55 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1520,7 +1520,9 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
{
int err;
- if (boot_cpu_has(X86_FEATURE_PSE))
+ if (end - start < PAGES_PER_SECTION * sizeof(struct page))
+ err = vmemmap_populate_basepages(start, end, node);
+ else if (boot_cpu_has(X86_FEATURE_PSE))
err = vmemmap_populate_hugepages(start, end, node, altmap);
else if (altmap) {
pr_err_once("%s: no cpu support for altmap allocations\n",
diff --git a/include/linux/mm.h b/include/linux/mm.h
index acc578407e9e..c502f3ce8661 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2734,8 +2734,8 @@ const char * arch_vma_name(struct vm_area_struct *vma);
void print_vma_addr(char *prefix, unsigned long rip);
void *sparse_buffer_alloc(unsigned long size);
-struct page *sparse_mem_map_populate(unsigned long pnum, int nid,
- struct vmem_altmap *altmap);
+struct page * __populate_section_memmap(unsigned long pfn,
+ unsigned long nr_pages, int nid, struct vmem_altmap *altmap);
pgd_t *vmemmap_pgd_populate(unsigned long addr, int node);
p4d_t *vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node);
pud_t *vmemmap_pud_populate(p4d_t *p4d, unsigned long addr, int node);
diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
index 7fec05796796..200aef686722 100644
--- a/mm/sparse-vmemmap.c
+++ b/mm/sparse-vmemmap.c
@@ -245,19 +245,26 @@ int __meminit vmemmap_populate_basepages(unsigned long start,
return 0;
}
-struct page * __meminit sparse_mem_map_populate(unsigned long pnum, int nid,
- struct vmem_altmap *altmap)
+struct page * __meminit __populate_section_memmap(unsigned long pfn,
+ unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
{
unsigned long start;
unsigned long end;
- struct page *map;
- map = pfn_to_page(pnum * PAGES_PER_SECTION);
- start = (unsigned long)map;
- end = (unsigned long)(map + PAGES_PER_SECTION);
+ /*
+ * The minimum granularity of memmap extensions is
+ * PAGES_PER_SUBSECTION as allocations are tracked in the
+ * 'subsection_map' bitmap of the section.
+ */
+ end = ALIGN(pfn + nr_pages, PAGES_PER_SUBSECTION);
+ pfn &= PAGE_SUBSECTION_MASK;
+ nr_pages = end - pfn;
+
+ start = (unsigned long) pfn_to_page(pfn);
+ end = start + nr_pages * sizeof(struct page);
if (vmemmap_populate(start, end, nid, altmap))
return NULL;
- return map;
+ return pfn_to_page(pfn);
}
diff --git a/mm/sparse.c b/mm/sparse.c
index 0baa2e55cfdd..2093c662a5f7 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -439,8 +439,8 @@ static unsigned long __init section_map_size(void)
return PAGE_ALIGN(sizeof(struct page) * PAGES_PER_SECTION);
}
-struct page __init *sparse_mem_map_populate(unsigned long pnum, int nid,
- struct vmem_altmap *altmap)
+struct page __init *__populate_section_memmap(unsigned long pfn,
+ unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
{
unsigned long size = section_map_size();
struct page *map = sparse_buffer_alloc(size);
@@ -521,10 +521,13 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,
}
sparse_buffer_init(map_count * section_map_size(), nid);
for_each_present_section_nr(pnum_begin, pnum) {
+ unsigned long pfn = section_nr_to_pfn(pnum);
+
if (pnum >= pnum_end)
break;
- map = sparse_mem_map_populate(pnum, nid, NULL);
+ map = __populate_section_memmap(pfn, PAGES_PER_SECTION,
+ nid, NULL);
if (!map) {
pr_err("%s: node[%d] memory map backing failed. Some memory will not be available.",
__func__, nid);
@@ -624,17 +627,17 @@ void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
#endif
#ifdef CONFIG_SPARSEMEM_VMEMMAP
-static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid,
- struct vmem_altmap *altmap)
+static struct page *populate_section_memmap(unsigned long pfn,
+ unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
{
- /* This will make the necessary allocations eventually. */
- return sparse_mem_map_populate(pnum, nid, altmap);
+ return __populate_section_memmap(pfn, nr_pages, nid, altmap);
}
-static void __kfree_section_memmap(struct page *memmap,
+
+static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,
struct vmem_altmap *altmap)
{
- unsigned long start = (unsigned long)memmap;
- unsigned long end = (unsigned long)(memmap + PAGES_PER_SECTION);
+ unsigned long start = (unsigned long) pfn_to_page(pfn);
+ unsigned long end = start + nr_pages * sizeof(struct page);
vmemmap_free(start, end, altmap);
}
@@ -646,11 +649,18 @@ static void free_map_bootmem(struct page *memmap)
vmemmap_free(start, end, NULL);
}
#else
-static struct page *__kmalloc_section_memmap(void)
+struct page *populate_section_memmap(unsigned long pfn,
+ unsigned long nr_pages, int nid, struct vmem_altmap *altmap)
{
struct page *page, *ret;
unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION;
+ if ((pfn & ~PAGE_SECTION_MASK) || nr_pages != PAGES_PER_SECTION) {
+ WARN(1, "%s: called with section unaligned parameters\n",
+ __func__);
+ return NULL;
+ }
+
page = alloc_pages(GFP_KERNEL|__GFP_NOWARN, get_order(memmap_size));
if (page)
goto got_map_page;
@@ -667,15 +677,17 @@ static struct page *__kmalloc_section_memmap(void)
return ret;
}
-static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid,
+static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,
struct vmem_altmap *altmap)
{
- return __kmalloc_section_memmap();
-}
+ struct page *memmap = pfn_to_page(pfn);
+
+ if ((pfn & ~PAGE_SECTION_MASK) || nr_pages != PAGES_PER_SECTION) {
+ WARN(1, "%s: called with section unaligned parameters\n",
+ __func__);
+ return;
+ }
-static void __kfree_section_memmap(struct page *memmap,
- struct vmem_altmap *altmap)
-{
if (is_vmalloc_addr(memmap))
vfree(memmap);
else
@@ -744,12 +756,13 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
if (ret < 0 && ret != -EEXIST)
return ret;
ret = 0;
- memmap = kmalloc_section_memmap(section_nr, nid, altmap);
+ memmap = populate_section_memmap(start_pfn, PAGES_PER_SECTION, nid,
+ altmap);
if (!memmap)
return -ENOMEM;
usage = kzalloc(mem_section_usage_size(), GFP_KERNEL);
if (!usage) {
- __kfree_section_memmap(memmap, altmap);
+ depopulate_section_memmap(start_pfn, PAGES_PER_SECTION, altmap);
return -ENOMEM;
}
@@ -771,7 +784,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
out:
if (ret < 0) {
kfree(usage);
- __kfree_section_memmap(memmap, altmap);
+ depopulate_section_memmap(start_pfn, PAGES_PER_SECTION, altmap);
}
return ret;
}
@@ -807,7 +820,8 @@ static inline void clear_hwpoisoned_pages(struct page *memmap, int nr_pages)
#endif
static void free_section_usage(struct page *memmap,
- struct mem_section_usage *usage, struct vmem_altmap *altmap)
+ struct mem_section_usage *usage, unsigned long pfn,
+ unsigned long nr_pages, struct vmem_altmap *altmap)
{
struct page *usage_page;
@@ -821,7 +835,7 @@ static void free_section_usage(struct page *memmap,
if (PageSlab(usage_page) || PageCompound(usage_page)) {
kfree(usage);
if (memmap)
- __kfree_section_memmap(memmap, altmap);
+ depopulate_section_memmap(pfn, nr_pages, altmap);
return;
}
@@ -850,6 +864,7 @@ void sparse_remove_one_section(struct mem_section *ms, unsigned long map_offset,
clear_hwpoisoned_pages(memmap + map_offset,
PAGES_PER_SECTION - map_offset);
- free_section_usage(memmap, usage, altmap);
+ free_section_usage(memmap, usage, section_nr_to_pfn(__section_nr(ms)),
+ PAGES_PER_SECTION, altmap);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
The zone type check was a leftover from the cleanup that plumbed altmap
through the memory hotplug path, i.e. commit da024512a1fa "mm: pass the
vmem_altmap to arch_remove_memory and __remove_pages".
Cc: Michal Hocko <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Cc: Pavel Tatashin <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
mm/memory_hotplug.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 647859a1d119..4b882c57781a 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -535,11 +535,8 @@ void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
unsigned long map_offset = 0;
int sections_to_remove;
- /* In the ZONE_DEVICE case device driver owns the memory region */
- if (is_dev_zone(zone)) {
- if (altmap)
- map_offset = vmem_altmap_offset(altmap);
- }
+ if (altmap)
+ map_offset = vmem_altmap_offset(altmap);
clear_zone_contiguous(zone);
Given there are no more usages of is_dev_zone() outside of 'ifdef
CONFIG_ZONE_DEVICE' protection, kill off the compilation helper.
Cc: Michal Hocko <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Acked-by: David Hildenbrand <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
Reviewed-by: Pavel Tatashin <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
include/linux/mmzone.h | 12 ------------
mm/page_alloc.c | 2 +-
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 6dd52d544857..49e7fb452dfd 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -855,18 +855,6 @@ static inline int local_memory_node(int node_id) { return node_id; };
*/
#define zone_idx(zone) ((zone) - (zone)->zone_pgdat->node_zones)
-#ifdef CONFIG_ZONE_DEVICE
-static inline bool is_dev_zone(const struct zone *zone)
-{
- return zone_idx(zone) == ZONE_DEVICE;
-}
-#else
-static inline bool is_dev_zone(const struct zone *zone)
-{
- return false;
-}
-#endif
-
/*
* Returns true if a zone has pages managed by the buddy allocator.
* All the reclaim decisions have to use this function rather than
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bd773efe5b82..5dff3f49a372 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5865,7 +5865,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
unsigned long start = jiffies;
int nid = pgdat->node_id;
- if (WARN_ON_ONCE(!pgmap || !is_dev_zone(zone)))
+ if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
return;
/*
Prepare the memory hot-{add,remove} paths for handling sub-section
ranges by plumbing the starting page frame and number of pages being
handled through arch_{add,remove}_memory() to
sparse_{add,remove}_one_section().
This is simply plumbing, small cleanups, and some identifier renames. No
intended functional changes.
Cc: Michal Hocko <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Cc: Oscar Salvador <[email protected]>
Reviewed-by: Pavel Tatashin <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
include/linux/memory_hotplug.h | 5 +-
mm/memory_hotplug.c | 114 +++++++++++++++++++++++++---------------
mm/sparse.c | 15 ++---
3 files changed, 81 insertions(+), 53 deletions(-)
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 79e0add6a597..3ab0282b4fe5 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -348,9 +348,10 @@ extern int add_memory_resource(int nid, struct resource *resource);
extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
unsigned long nr_pages, struct vmem_altmap *altmap);
extern bool is_memblock_offlined(struct memory_block *mem);
-extern int sparse_add_one_section(int nid, unsigned long start_pfn,
- struct vmem_altmap *altmap);
+extern int sparse_add_section(int nid, unsigned long pfn,
+ unsigned long nr_pages, struct vmem_altmap *altmap);
extern void sparse_remove_one_section(struct mem_section *ms,
+ unsigned long pfn, unsigned long nr_pages,
unsigned long map_offset, struct vmem_altmap *altmap);
extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
unsigned long pnum);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 4b882c57781a..399bf78bccc5 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -252,51 +252,84 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
}
#endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
-static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
- struct vmem_altmap *altmap)
+static int __meminit __add_section(int nid, unsigned long pfn,
+ unsigned long nr_pages, struct vmem_altmap *altmap)
{
int ret;
- if (pfn_valid(phys_start_pfn))
+ if (pfn_valid(pfn))
return -EEXIST;
- ret = sparse_add_one_section(nid, phys_start_pfn, altmap);
+ ret = sparse_add_section(nid, pfn, nr_pages, altmap);
return ret < 0 ? ret : 0;
}
+static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
+ const char *reason)
+{
+ /*
+ * Disallow all operations smaller than a sub-section and only
+ * allow operations smaller than a section for
+ * SPARSEMEM_VMEMMAP. Note that check_hotplug_memory_range()
+ * enforces a larger memory_block_size_bytes() granularity for
+ * memory that will be marked online, so this check should only
+ * fire for direct arch_{add,remove}_memory() users outside of
+ * add_memory_resource().
+ */
+ unsigned long min_align;
+
+ if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
+ min_align = PAGES_PER_SUBSECTION;
+ else
+ min_align = PAGES_PER_SECTION;
+ if (!IS_ALIGNED(pfn, min_align)
+ || !IS_ALIGNED(nr_pages, min_align)) {
+ WARN(1, "Misaligned __%s_pages start: %#lx end: #%lx\n",
+ reason, pfn, pfn + nr_pages - 1);
+ return -EINVAL;
+ }
+ return 0;
+}
+
/*
* Reasonably generic function for adding memory. It is
* expected that archs that support memory hotplug will
* call this function after deciding the zone to which to
* add the new pages.
*/
-int __ref __add_pages(int nid, unsigned long phys_start_pfn,
- unsigned long nr_pages, struct mhp_restrictions *restrictions)
+int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
+ struct mhp_restrictions *restrictions)
{
unsigned long i;
- int err = 0;
- int start_sec, end_sec;
+ int start_sec, end_sec, err;
struct vmem_altmap *altmap = restrictions->altmap;
- /* during initialize mem_map, align hot-added range to section */
- start_sec = pfn_to_section_nr(phys_start_pfn);
- end_sec = pfn_to_section_nr(phys_start_pfn + nr_pages - 1);
-
if (altmap) {
/*
* Validate altmap is within bounds of the total request
*/
- if (altmap->base_pfn != phys_start_pfn
+ if (altmap->base_pfn != pfn
|| vmem_altmap_offset(altmap) > nr_pages) {
pr_warn_once("memory add fail, invalid altmap\n");
- err = -EINVAL;
- goto out;
+ return -EINVAL;
}
altmap->alloc = 0;
}
+ err = check_pfn_span(pfn, nr_pages, "add");
+ if (err)
+ return err;
+
+ start_sec = pfn_to_section_nr(pfn);
+ end_sec = pfn_to_section_nr(pfn + nr_pages - 1);
for (i = start_sec; i <= end_sec; i++) {
- err = __add_section(nid, section_nr_to_pfn(i), altmap);
+ unsigned long pfns;
+
+ pfns = min(nr_pages, PAGES_PER_SECTION
+ - (pfn & ~PAGE_SECTION_MASK));
+ err = __add_section(nid, pfn, pfns, altmap);
+ pfn += pfns;
+ nr_pages -= pfns;
/*
* EEXIST is finally dealt with by ioresource collision
@@ -309,7 +342,6 @@ int __ref __add_pages(int nid, unsigned long phys_start_pfn,
cond_resched();
}
vmemmap_populate_print_last();
-out:
return err;
}
@@ -487,10 +519,10 @@ static void shrink_pgdat_span(struct pglist_data *pgdat,
pgdat->node_spanned_pages = 0;
}
-static void __remove_zone(struct zone *zone, unsigned long start_pfn)
+static void __remove_zone(struct zone *zone, unsigned long start_pfn,
+ unsigned long nr_pages)
{
struct pglist_data *pgdat = zone->zone_pgdat;
- int nr_pages = PAGES_PER_SECTION;
unsigned long flags;
pgdat_resize_lock(zone->zone_pgdat, &flags);
@@ -499,27 +531,23 @@ static void __remove_zone(struct zone *zone, unsigned long start_pfn)
pgdat_resize_unlock(zone->zone_pgdat, &flags);
}
-static void __remove_section(struct zone *zone, struct mem_section *ms,
- unsigned long map_offset,
- struct vmem_altmap *altmap)
+static void __remove_section(struct zone *zone, unsigned long pfn,
+ unsigned long nr_pages, unsigned long map_offset,
+ struct vmem_altmap *altmap)
{
- unsigned long start_pfn;
- int scn_nr;
+ struct mem_section *ms = __nr_to_section(pfn_to_section_nr(pfn));
if (WARN_ON_ONCE(!valid_section(ms)))
return;
- scn_nr = __section_nr(ms);
- start_pfn = section_nr_to_pfn((unsigned long)scn_nr);
- __remove_zone(zone, start_pfn);
-
- sparse_remove_one_section(ms, map_offset, altmap);
+ __remove_zone(zone, pfn, nr_pages);
+ sparse_remove_one_section(ms, pfn, nr_pages, map_offset, altmap);
}
/**
* __remove_pages() - remove sections of pages from a zone
* @zone: zone from which pages need to be removed
- * @phys_start_pfn: starting pageframe (must be aligned to start of a section)
+ * @pfn: starting pageframe (must be aligned to start of a section)
* @nr_pages: number of pages to remove (must be multiple of section size)
* @altmap: alternative device page map or %NULL if default memmap is used
*
@@ -528,31 +556,31 @@ static void __remove_section(struct zone *zone, struct mem_section *ms,
* sure that pages are marked reserved and zones are adjust properly by
* calling offline_pages().
*/
-void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
+void __remove_pages(struct zone *zone, unsigned long pfn,
unsigned long nr_pages, struct vmem_altmap *altmap)
{
- unsigned long i;
unsigned long map_offset = 0;
- int sections_to_remove;
+ int i, start_sec, end_sec;
if (altmap)
map_offset = vmem_altmap_offset(altmap);
clear_zone_contiguous(zone);
- /*
- * We can only remove entire sections
- */
- BUG_ON(phys_start_pfn & ~PAGE_SECTION_MASK);
- BUG_ON(nr_pages % PAGES_PER_SECTION);
+ if (check_pfn_span(pfn, nr_pages, "remove"))
+ return;
- sections_to_remove = nr_pages / PAGES_PER_SECTION;
- for (i = 0; i < sections_to_remove; i++) {
- unsigned long pfn = phys_start_pfn + i*PAGES_PER_SECTION;
+ start_sec = pfn_to_section_nr(pfn);
+ end_sec = pfn_to_section_nr(pfn + nr_pages - 1);
+ for (i = start_sec; i <= end_sec; i++) {
+ unsigned long pfns;
cond_resched();
- __remove_section(zone, __pfn_to_section(pfn), map_offset,
- altmap);
+ pfns = min(nr_pages, PAGES_PER_SECTION
+ - (pfn & ~PAGE_SECTION_MASK));
+ __remove_section(zone, pfn, pfns, map_offset, altmap);
+ pfn += pfns;
+ nr_pages -= pfns;
map_offset = 0;
}
diff --git a/mm/sparse.c b/mm/sparse.c
index 2093c662a5f7..f65206deaf49 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -739,8 +739,8 @@ static void free_map_bootmem(struct page *memmap)
* * -EEXIST - Section has been present.
* * -ENOMEM - Out of memory.
*/
-int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
- struct vmem_altmap *altmap)
+int __meminit sparse_add_section(int nid, unsigned long start_pfn,
+ unsigned long nr_pages, struct vmem_altmap *altmap)
{
unsigned long section_nr = pfn_to_section_nr(start_pfn);
struct mem_section_usage *usage;
@@ -848,8 +848,9 @@ static void free_section_usage(struct page *memmap,
free_map_bootmem(memmap);
}
-void sparse_remove_one_section(struct mem_section *ms, unsigned long map_offset,
- struct vmem_altmap *altmap)
+void sparse_remove_one_section(struct mem_section *ms, unsigned long pfn,
+ unsigned long nr_pages, unsigned long map_offset,
+ struct vmem_altmap *altmap)
{
struct page *memmap = NULL;
struct mem_section_usage *usage = NULL;
@@ -862,9 +863,7 @@ void sparse_remove_one_section(struct mem_section *ms, unsigned long map_offset,
ms->usage = NULL;
}
- clear_hwpoisoned_pages(memmap + map_offset,
- PAGES_PER_SECTION - map_offset);
- free_section_usage(memmap, usage, section_nr_to_pfn(__section_nr(ms)),
- PAGES_PER_SECTION, altmap);
+ clear_hwpoisoned_pages(memmap + map_offset, nr_pages - map_offset);
+ free_section_usage(memmap, usage, pfn, nr_pages, altmap);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
The libnvdimm sub-system has suffered a series of hacks and broken
workarounds for the memory-hotplug implementation's awkward
section-aligned (128MB) granularity. For example the following backtrace
is emitted when attempting arch_add_memory() with physical address
ranges that intersect 'System RAM' (RAM) with 'Persistent Memory' (PMEM)
within a given section:
WARNING: CPU: 0 PID: 558 at kernel/memremap.c:300 devm_memremap_pages+0x3b5/0x4c0
devm_memremap_pages attempted on mixed region [mem 0x200000000-0x2fbffffff flags 0x200]
[..]
Call Trace:
dump_stack+0x86/0xc3
__warn+0xcb/0xf0
warn_slowpath_fmt+0x5f/0x80
devm_memremap_pages+0x3b5/0x4c0
__wrap_devm_memremap_pages+0x58/0x70 [nfit_test_iomap]
pmem_attach_disk+0x19a/0x440 [nd_pmem]
Recently it was discovered that the problem goes beyond RAM vs PMEM
collisions as some platform produce PMEM vs PMEM collisions within a
given section. The libnvdimm workaround for that case revealed that the
libnvdimm section-alignment-padding implementation has been broken for a
long while. A fix for that long-standing breakage introduces as many
problems as it solves as it would require a backward-incompatible change
to the namespace metadata interpretation. Instead of that dubious route
[1], address the root problem in the memory-hotplug implementation.
[1]: https://lore.kernel.org/r/155000671719.348031.2347363160141119237.stgit@dwillia2-desk3.amr.corp.intel.com
Cc: Michal Hocko <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Pavel Tatashin <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
include/linux/memory_hotplug.h | 2
mm/memory_hotplug.c | 7 -
mm/page_alloc.c | 2
mm/sparse.c | 225 +++++++++++++++++++++++++++-------------
4 files changed, 155 insertions(+), 81 deletions(-)
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 3ab0282b4fe5..0b8a5e5ef2da 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -350,7 +350,7 @@ extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
extern bool is_memblock_offlined(struct memory_block *mem);
extern int sparse_add_section(int nid, unsigned long pfn,
unsigned long nr_pages, struct vmem_altmap *altmap);
-extern void sparse_remove_one_section(struct mem_section *ms,
+extern void sparse_remove_section(struct mem_section *ms,
unsigned long pfn, unsigned long nr_pages,
unsigned long map_offset, struct vmem_altmap *altmap);
extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 399bf78bccc5..8188be7a9edb 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -255,13 +255,10 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
static int __meminit __add_section(int nid, unsigned long pfn,
unsigned long nr_pages, struct vmem_altmap *altmap)
{
- int ret;
-
if (pfn_valid(pfn))
return -EEXIST;
- ret = sparse_add_section(nid, pfn, nr_pages, altmap);
- return ret < 0 ? ret : 0;
+ return sparse_add_section(nid, pfn, nr_pages, altmap);
}
static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
@@ -541,7 +538,7 @@ static void __remove_section(struct zone *zone, unsigned long pfn,
return;
__remove_zone(zone, pfn, nr_pages);
- sparse_remove_one_section(ms, pfn, nr_pages, map_offset, altmap);
+ sparse_remove_section(ms, pfn, nr_pages, map_offset, altmap);
}
/**
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5dff3f49a372..af260cc469cd 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5915,7 +5915,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
* pfn out of zone.
*
* Please note that MEMMAP_HOTPLUG path doesn't clear memmap
- * because this is done early in sparse_add_one_section
+ * because this is done early in section_activate()
*/
if (!(pfn & (pageblock_nr_pages - 1))) {
set_pageblock_migratetype(page, MIGRATE_MOVABLE);
diff --git a/mm/sparse.c b/mm/sparse.c
index f65206deaf49..d83bac5d1324 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -83,8 +83,15 @@ static int __meminit sparse_index_init(unsigned long section_nr, int nid)
unsigned long root = SECTION_NR_TO_ROOT(section_nr);
struct mem_section *section;
+ /*
+ * An existing section is possible in the sub-section hotplug
+ * case. First hot-add instantiates, follow-on hot-add reuses
+ * the existing section.
+ *
+ * The mem_hotplug_lock resolves the apparent race below.
+ */
if (mem_section[root])
- return -EEXIST;
+ return 0;
section = sparse_index_alloc(nid);
if (!section)
@@ -325,6 +332,15 @@ static void __meminit sparse_init_one_section(struct mem_section *ms,
unsigned long pnum, struct page *mem_map,
struct mem_section_usage *usage)
{
+ /*
+ * Given that SPARSEMEM_VMEMMAP=y supports sub-section hotplug,
+ * ->section_mem_map can not be guaranteed to point to a full
+ * section's worth of memory. The field is only valid / used
+ * in the SPARSEMEM_VMEMMAP=n case.
+ */
+ if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
+ mem_map = NULL;
+
ms->section_mem_map &= ~SECTION_MAP_MASK;
ms->section_mem_map |= sparse_encode_mem_map(mem_map, pnum) |
SECTION_HAS_MEM_MAP;
@@ -726,10 +742,131 @@ static void free_map_bootmem(struct page *memmap)
}
#endif /* CONFIG_SPARSEMEM_VMEMMAP */
+static bool is_early_section(struct mem_section *ms)
+{
+ struct page *usage_page;
+
+ usage_page = virt_to_page(ms->usage);
+ if (PageSlab(usage_page) || PageCompound(usage_page))
+ return false;
+ else
+ return true;
+}
+
+static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
+ struct vmem_altmap *altmap)
+{
+ DECLARE_BITMAP(map, SUBSECTIONS_PER_SECTION) = { 0 };
+ DECLARE_BITMAP(tmp, SUBSECTIONS_PER_SECTION) = { 0 };
+ struct mem_section *ms = __pfn_to_section(pfn);
+ bool early_section = is_early_section(ms);
+ struct page *memmap = NULL;
+ unsigned long *subsection_map = ms->usage
+ ? &ms->usage->subsection_map[0] : NULL;
+
+ subsection_mask_set(map, pfn, nr_pages);
+ if (subsection_map)
+ bitmap_and(tmp, map, subsection_map, SUBSECTIONS_PER_SECTION);
+
+ if (WARN(!subsection_map || !bitmap_equal(tmp, map, SUBSECTIONS_PER_SECTION),
+ "section already deactivated (%#lx + %ld)\n",
+ pfn, nr_pages))
+ return;
+
+ /*
+ * There are 3 cases to handle across two configurations
+ * (SPARSEMEM_VMEMMAP={y,n}):
+ *
+ * 1/ deactivation of a partial hot-added section (only possible
+ * in the SPARSEMEM_VMEMMAP=y case).
+ * a/ section was present at memory init
+ * b/ section was hot-added post memory init
+ * 2/ deactivation of a complete hot-added section
+ * 3/ deactivation of a complete section from memory init
+ *
+ * For 1/, when subsection_map does not empty we will not be
+ * freeing the usage map, but still need to free the vmemmap
+ * range.
+ *
+ * For 2/ and 3/ the SPARSEMEM_VMEMMAP={y,n} cases are unified
+ */
+ bitmap_xor(subsection_map, map, subsection_map, SUBSECTIONS_PER_SECTION);
+ if (bitmap_empty(subsection_map, SUBSECTIONS_PER_SECTION)) {
+ unsigned long section_nr = pfn_to_section_nr(pfn);
+
+ if (!early_section) {
+ kfree(ms->usage);
+ ms->usage = NULL;
+ }
+ memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
+ ms->section_mem_map = sparse_encode_mem_map(NULL, section_nr);
+ }
+
+ if (early_section && memmap)
+ free_map_bootmem(memmap);
+ else
+ depopulate_section_memmap(pfn, nr_pages, altmap);
+}
+
+static struct page * __meminit section_activate(int nid, unsigned long pfn,
+ unsigned long nr_pages, struct vmem_altmap *altmap)
+{
+ DECLARE_BITMAP(map, SUBSECTIONS_PER_SECTION) = { 0 };
+ struct mem_section *ms = __pfn_to_section(pfn);
+ struct mem_section_usage *usage = NULL;
+ unsigned long *subsection_map;
+ struct page *memmap;
+ int rc = 0;
+
+ subsection_mask_set(map, pfn, nr_pages);
+
+ if (!ms->usage) {
+ usage = kzalloc(mem_section_usage_size(), GFP_KERNEL);
+ if (!usage)
+ return ERR_PTR(-ENOMEM);
+ ms->usage = usage;
+ }
+ subsection_map = &ms->usage->subsection_map[0];
+
+ if (bitmap_empty(map, SUBSECTIONS_PER_SECTION))
+ rc = -EINVAL;
+ else if (bitmap_intersects(map, subsection_map, SUBSECTIONS_PER_SECTION))
+ rc = -EEXIST;
+ else
+ bitmap_or(subsection_map, map, subsection_map,
+ SUBSECTIONS_PER_SECTION);
+
+ if (rc) {
+ if (usage)
+ ms->usage = NULL;
+ kfree(usage);
+ return ERR_PTR(rc);
+ }
+
+ /*
+ * The early init code does not consider partially populated
+ * initial sections, it simply assumes that memory will never be
+ * referenced. If we hot-add memory into such a section then we
+ * do not need to populate the memmap and can simply reuse what
+ * is already there.
+ */
+ if (nr_pages < PAGES_PER_SECTION && is_early_section(ms))
+ return pfn_to_page(pfn);
+
+ memmap = populate_section_memmap(pfn, nr_pages, nid, altmap);
+ if (!memmap) {
+ section_deactivate(pfn, nr_pages, altmap);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ return memmap;
+}
+
/**
- * sparse_add_one_section - add a memory section
+ * sparse_add_section - add a memory section, or populate an existing one
* @nid: The node to add section on
* @start_pfn: start pfn of the memory range
+ * @nr_pages: number of pfns to add in the section
* @altmap: device page map
*
* This is only intended for hotplug.
@@ -743,50 +880,29 @@ int __meminit sparse_add_section(int nid, unsigned long start_pfn,
unsigned long nr_pages, struct vmem_altmap *altmap)
{
unsigned long section_nr = pfn_to_section_nr(start_pfn);
- struct mem_section_usage *usage;
struct mem_section *ms;
struct page *memmap;
int ret;
- /*
- * no locking for this, because it does its own
- * plus, it does a kmalloc
- */
ret = sparse_index_init(section_nr, nid);
- if (ret < 0 && ret != -EEXIST)
+ if (ret < 0)
return ret;
- ret = 0;
- memmap = populate_section_memmap(start_pfn, PAGES_PER_SECTION, nid,
- altmap);
- if (!memmap)
- return -ENOMEM;
- usage = kzalloc(mem_section_usage_size(), GFP_KERNEL);
- if (!usage) {
- depopulate_section_memmap(start_pfn, PAGES_PER_SECTION, altmap);
- return -ENOMEM;
- }
- ms = __pfn_to_section(start_pfn);
- if (ms->section_mem_map & SECTION_MARKED_PRESENT) {
- ret = -EEXIST;
- goto out;
- }
+ memmap = section_activate(nid, start_pfn, nr_pages, altmap);
+ if (IS_ERR(memmap))
+ return PTR_ERR(memmap);
/*
* Poison uninitialized struct pages in order to catch invalid flags
* combinations.
*/
- page_init_poison(memmap, sizeof(struct page) * PAGES_PER_SECTION);
+ page_init_poison(pfn_to_page(start_pfn), sizeof(struct page) * nr_pages);
+ ms = __pfn_to_section(start_pfn);
section_mark_present(ms);
- sparse_init_one_section(ms, section_nr, memmap, usage);
+ sparse_init_one_section(ms, section_nr, memmap, ms->usage);
-out:
- if (ret < 0) {
- kfree(usage);
- depopulate_section_memmap(start_pfn, PAGES_PER_SECTION, altmap);
- }
- return ret;
+ return 0;
}
#ifdef CONFIG_MEMORY_FAILURE
@@ -819,51 +935,12 @@ static inline void clear_hwpoisoned_pages(struct page *memmap, int nr_pages)
}
#endif
-static void free_section_usage(struct page *memmap,
- struct mem_section_usage *usage, unsigned long pfn,
- unsigned long nr_pages, struct vmem_altmap *altmap)
-{
- struct page *usage_page;
-
- if (!usage)
- return;
-
- usage_page = virt_to_page(usage);
- /*
- * Check to see if allocation came from hot-plug-add
- */
- if (PageSlab(usage_page) || PageCompound(usage_page)) {
- kfree(usage);
- if (memmap)
- depopulate_section_memmap(pfn, nr_pages, altmap);
- return;
- }
-
- /*
- * The usemap came from bootmem. This is packed with other usemaps
- * on the section which has pgdat at boot time. Just keep it as is now.
- */
-
- if (memmap)
- free_map_bootmem(memmap);
-}
-
-void sparse_remove_one_section(struct mem_section *ms, unsigned long pfn,
+void sparse_remove_section(struct mem_section *ms, unsigned long pfn,
unsigned long nr_pages, unsigned long map_offset,
struct vmem_altmap *altmap)
{
- struct page *memmap = NULL;
- struct mem_section_usage *usage = NULL;
-
- if (ms->section_mem_map) {
- usage = ms->usage;
- memmap = sparse_decode_mem_map(ms->section_mem_map,
- __section_nr(ms));
- ms->section_mem_map = 0;
- ms->usage = NULL;
- }
-
- clear_hwpoisoned_pages(memmap + map_offset, nr_pages - map_offset);
- free_section_usage(memmap, usage, pfn, nr_pages, altmap);
+ clear_hwpoisoned_pages(pfn_to_page(pfn) + map_offset,
+ nr_pages - map_offset);
+ section_deactivate(pfn, nr_pages, altmap);
}
#endif /* CONFIG_MEMORY_HOTPLUG */
Sub-section hotplug support reduces the unit of operation of hotplug
from section-sized-units (PAGES_PER_SECTION) to sub-section-sized units
(PAGES_PER_SUBSECTION). Teach shrink_{zone,pgdat}_span() to consider
PAGES_PER_SUBSECTION boundaries as the points where pfn_valid(), not
valid_section(), can toggle.
Cc: Michal Hocko <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Reviewed-by: Pavel Tatashin <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
mm/memory_hotplug.c | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 7b963c2d3a0d..647859a1d119 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -318,12 +318,8 @@ static unsigned long find_smallest_section_pfn(int nid, struct zone *zone,
unsigned long start_pfn,
unsigned long end_pfn)
{
- struct mem_section *ms;
-
- for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SECTION) {
- ms = __pfn_to_section(start_pfn);
-
- if (unlikely(!valid_section(ms)))
+ for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SUBSECTION) {
+ if (unlikely(!pfn_valid(start_pfn)))
continue;
if (unlikely(pfn_to_nid(start_pfn) != nid))
@@ -343,15 +339,12 @@ static unsigned long find_biggest_section_pfn(int nid, struct zone *zone,
unsigned long start_pfn,
unsigned long end_pfn)
{
- struct mem_section *ms;
unsigned long pfn;
/* pfn is the end pfn of a memory section. */
pfn = end_pfn - 1;
- for (; pfn >= start_pfn; pfn -= PAGES_PER_SECTION) {
- ms = __pfn_to_section(pfn);
-
- if (unlikely(!valid_section(ms)))
+ for (; pfn >= start_pfn; pfn -= PAGES_PER_SUBSECTION) {
+ if (unlikely(!pfn_valid(pfn)))
continue;
if (unlikely(pfn_to_nid(pfn) != nid))
@@ -373,7 +366,6 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
unsigned long z = zone_end_pfn(zone); /* zone_end_pfn namespace clash */
unsigned long zone_end_pfn = z;
unsigned long pfn;
- struct mem_section *ms;
int nid = zone_to_nid(zone);
zone_span_writelock(zone);
@@ -410,10 +402,8 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
* it check the zone has only hole or not.
*/
pfn = zone_start_pfn;
- for (; pfn < zone_end_pfn; pfn += PAGES_PER_SECTION) {
- ms = __pfn_to_section(pfn);
-
- if (unlikely(!valid_section(ms)))
+ for (; pfn < zone_end_pfn; pfn += PAGES_PER_SUBSECTION) {
+ if (unlikely(!pfn_valid(pfn)))
continue;
if (page_zone(pfn_to_page(pfn)) != zone)
@@ -441,7 +431,6 @@ static void shrink_pgdat_span(struct pglist_data *pgdat,
unsigned long p = pgdat_end_pfn(pgdat); /* pgdat_end_pfn namespace clash */
unsigned long pgdat_end_pfn = p;
unsigned long pfn;
- struct mem_section *ms;
int nid = pgdat->node_id;
if (pgdat_start_pfn == start_pfn) {
@@ -478,10 +467,8 @@ static void shrink_pgdat_span(struct pglist_data *pgdat,
* has only hole or not.
*/
pfn = pgdat_start_pfn;
- for (; pfn < pgdat_end_pfn; pfn += PAGES_PER_SECTION) {
- ms = __pfn_to_section(pfn);
-
- if (unlikely(!valid_section(ms)))
+ for (; pfn < pgdat_end_pfn; pfn += PAGES_PER_SUBSECTION) {
+ if (unlikely(!pfn_valid(pfn)))
continue;
if (pfn_to_nid(pfn) != nid)
Teach devm_memremap_pages() about the new sub-section capabilities of
arch_{add,remove}_memory(). Effectively, just replace all usage of
align_start, align_end, and align_size with res->start, res->end, and
resource_size(res). The existing sanity check will still make sure that
the two separate remap attempts do not collide within a sub-section (2MB
on x86).
Cc: Michal Hocko <[email protected]>
Cc: Toshi Kani <[email protected]>
Cc: Jérôme Glisse <[email protected]>
Cc: Logan Gunthorpe <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Pavel Tatashin <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
kernel/memremap.c | 61 +++++++++++++++++++++--------------------------------
1 file changed, 24 insertions(+), 37 deletions(-)
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 57980ed4e571..a0e5f6b91b04 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -58,7 +58,7 @@ static unsigned long pfn_first(struct dev_pagemap *pgmap)
struct vmem_altmap *altmap = &pgmap->altmap;
unsigned long pfn;
- pfn = res->start >> PAGE_SHIFT;
+ pfn = PHYS_PFN(res->start);
if (pgmap->altmap_valid)
pfn += vmem_altmap_offset(altmap);
return pfn;
@@ -86,7 +86,6 @@ static void devm_memremap_pages_release(void *data)
struct dev_pagemap *pgmap = data;
struct device *dev = pgmap->dev;
struct resource *res = &pgmap->res;
- resource_size_t align_start, align_size;
unsigned long pfn;
int nid;
@@ -96,25 +95,21 @@ static void devm_memremap_pages_release(void *data)
pgmap->cleanup(pgmap->ref);
/* pages are dead and unused, undo the arch mapping */
- align_start = res->start & ~(PA_SECTION_SIZE - 1);
- align_size = ALIGN(res->start + resource_size(res), PA_SECTION_SIZE)
- - align_start;
-
- nid = page_to_nid(pfn_to_page(align_start >> PAGE_SHIFT));
+ nid = page_to_nid(pfn_to_page(PHYS_PFN(res->start)));
mem_hotplug_begin();
if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
- pfn = align_start >> PAGE_SHIFT;
+ pfn = PHYS_PFN(res->start);
__remove_pages(page_zone(pfn_to_page(pfn)), pfn,
- align_size >> PAGE_SHIFT, NULL);
+ PHYS_PFN(resource_size(res)), NULL);
} else {
- arch_remove_memory(nid, align_start, align_size,
+ arch_remove_memory(nid, res->start, resource_size(res),
pgmap->altmap_valid ? &pgmap->altmap : NULL);
- kasan_remove_zero_shadow(__va(align_start), align_size);
+ kasan_remove_zero_shadow(__va(res->start), resource_size(res));
}
mem_hotplug_done();
- untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
+ untrack_pfn(NULL, PHYS_PFN(res->start), resource_size(res));
pgmap_array_delete(res);
dev_WARN_ONCE(dev, pgmap->altmap.alloc,
"%s: failed to free all reserved pages\n", __func__);
@@ -141,16 +136,13 @@ static void devm_memremap_pages_release(void *data)
*/
void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
{
- resource_size_t align_start, align_size, align_end;
- struct vmem_altmap *altmap = pgmap->altmap_valid ?
- &pgmap->altmap : NULL;
struct resource *res = &pgmap->res;
struct dev_pagemap *conflict_pgmap;
struct mhp_restrictions restrictions = {
/*
* We do not want any optional features only our own memmap
*/
- .altmap = altmap,
+ .altmap = pgmap->altmap_valid ? &pgmap->altmap : NULL,
};
pgprot_t pgprot = PAGE_KERNEL;
int error, nid, is_ram;
@@ -160,12 +152,7 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
return ERR_PTR(-EINVAL);
}
- align_start = res->start & ~(PA_SECTION_SIZE - 1);
- align_size = ALIGN(res->start + resource_size(res), PA_SECTION_SIZE)
- - align_start;
- align_end = align_start + align_size - 1;
-
- conflict_pgmap = get_dev_pagemap(PHYS_PFN(align_start), NULL);
+ conflict_pgmap = get_dev_pagemap(PHYS_PFN(res->start), NULL);
if (conflict_pgmap) {
dev_WARN(dev, "Conflicting mapping in same section\n");
put_dev_pagemap(conflict_pgmap);
@@ -173,7 +160,7 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
goto err_array;
}
- conflict_pgmap = get_dev_pagemap(PHYS_PFN(align_end), NULL);
+ conflict_pgmap = get_dev_pagemap(PHYS_PFN(res->end), NULL);
if (conflict_pgmap) {
dev_WARN(dev, "Conflicting mapping in same section\n");
put_dev_pagemap(conflict_pgmap);
@@ -181,7 +168,7 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
goto err_array;
}
- is_ram = region_intersects(align_start, align_size,
+ is_ram = region_intersects(res->start, resource_size(res),
IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE);
if (is_ram != REGION_DISJOINT) {
@@ -202,8 +189,8 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
if (nid < 0)
nid = numa_mem_id();
- error = track_pfn_remap(NULL, &pgprot, PHYS_PFN(align_start), 0,
- align_size);
+ error = track_pfn_remap(NULL, &pgprot, PHYS_PFN(res->start), 0,
+ resource_size(res));
if (error)
goto err_pfn_remap;
@@ -221,25 +208,25 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
* arch_add_memory().
*/
if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
- error = add_pages(nid, align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, &restrictions);
+ error = add_pages(nid, PHYS_PFN(res->start),
+ PHYS_PFN(resource_size(res)), &restrictions);
} else {
- error = kasan_add_zero_shadow(__va(align_start), align_size);
+ error = kasan_add_zero_shadow(__va(res->start), resource_size(res));
if (error) {
mem_hotplug_done();
goto err_kasan;
}
- error = arch_add_memory(nid, align_start, align_size,
- &restrictions);
+ error = arch_add_memory(nid, res->start, resource_size(res),
+ &restrictions);
}
if (!error) {
struct zone *zone;
zone = &NODE_DATA(nid)->node_zones[ZONE_DEVICE];
- move_pfn_range_to_zone(zone, align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, altmap);
+ move_pfn_range_to_zone(zone, PHYS_PFN(res->start),
+ PHYS_PFN(resource_size(res)), restrictions.altmap);
}
mem_hotplug_done();
@@ -251,8 +238,8 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
* to allow us to do the work while not holding the hotplug lock.
*/
memmap_init_zone_device(&NODE_DATA(nid)->node_zones[ZONE_DEVICE],
- align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, pgmap);
+ PHYS_PFN(res->start),
+ PHYS_PFN(resource_size(res)), pgmap);
percpu_ref_get_many(pgmap->ref, pfn_end(pgmap) - pfn_first(pgmap));
error = devm_add_action_or_reset(dev, devm_memremap_pages_release,
@@ -263,9 +250,9 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
return __va(res->start);
err_add_memory:
- kasan_remove_zero_shadow(__va(align_start), align_size);
+ kasan_remove_zero_shadow(__va(res->start), resource_size(res));
err_kasan:
- untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
+ untrack_pfn(NULL, PHYS_PFN(res->start), resource_size(res));
err_pfn_remap:
pgmap_array_delete(res);
err_array:
Explain the general mechanisms of 'ZONE_DEVICE' pages and list the users
of 'devm_memremap_pages()'.
Cc: Jonathan Corbet <[email protected]>
Reported-by: Mike Rapoport <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
Documentation/vm/memory-model.rst | 39 +++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/Documentation/vm/memory-model.rst b/Documentation/vm/memory-model.rst
index 382f72ace1fc..e0af47e02e78 100644
--- a/Documentation/vm/memory-model.rst
+++ b/Documentation/vm/memory-model.rst
@@ -181,3 +181,42 @@ that is eventually passed to vmemmap_populate() through a long chain
of function calls. The vmemmap_populate() implementation may use the
`vmem_altmap` along with :c:func:`altmap_alloc_block_buf` helper to
allocate memory map on the persistent memory device.
+
+ZONE_DEVICE
+===========
+The `ZONE_DEVICE` facility builds upon `SPARSEMEM_VMEMMAP` to offer
+`struct page` `mem_map` services for device driver identified physical
+address ranges. The "device" aspect of `ZONE_DEVICE` relates to the fact
+that the page objects for these address ranges are never marked online,
+and that a reference must be taken against the device, not just the page
+to keep the memory pinned for active use. `ZONE_DEVICE`, via
+:c:func:`devm_memremap_pages`, performs just enough memory hotplug to
+turn on :c:func:`pfn_to_page`, :c:func:`page_to_pfn`, and
+:c:func:`get_user_pages` service for the given range of pfns. Since the
+page reference count never drops below 1 the page is never tracked as
+free memory and the page's `struct list_head lru` space is repurposed
+for back referencing to the host device / driver that mapped the memory.
+
+While `SPARSEMEM` presents memory as a collection of sections,
+optionally collected into memory blocks, `ZONE_DEVICE` users have a need
+for smaller granularity of populating the `mem_map`. Given that
+`ZONE_DEVICE` memory is never marked online it is subsequently never
+subject to its memory ranges being exposed through the sysfs memory
+hotplug api on memory block boundaries. The implementation relies on
+this lack of user-api constraint to allow sub-section sized memory
+ranges to be specified to :c:func:`arch_add_memory`, the top-half of
+memory hotplug. Sub-section support allows for `PMD_SIZE` as the minimum
+alignment granularity for :c:func:`devm_memremap_pages`.
+
+The users of `ZONE_DEVICE` are:
+* pmem: Map platform persistent memory to be used as a direct-I/O target
+ via DAX mappings.
+
+* hmm: Extend `ZONE_DEVICE` with `->page_fault()` and `->page_free()`
+ event callbacks to allow a device-driver to coordinate memory management
+ events related to device-memory, typically GPU memory. See
+ Documentation/vm/hmm.rst.
+
+* p2pdma: Create `struct page` objects to allow peer devices in a
+ PCI/-E topology to coordinate direct-DMA operations between themselves,
+ i.e. bypass host memory.
At namespace creation time there is the potential for the "expected to
be zero" fields of a 'pfn' info-block to be filled with indeterminate
data. While the kernel buffer is zeroed on allocation it is immediately
overwritten by nd_pfn_validate() filling it with the current contents of
the on-media info-block location. For fields like, 'flags' and the
'padding' it potentially means that future implementations can not rely
on those fields being zero.
In preparation to stop using the 'start_pad' and 'end_trunc' fields for
section alignment, arrange for fields that are not explicitly
initialized to be guaranteed zero. Bump the minor version to indicate it
is safe to assume the 'padding' and 'flags' are zero. Otherwise, this
corruption is expected to benign since all other critical fields are
explicitly initialized.
Fixes: 32ab0a3f5170 ("libnvdimm, pmem: 'struct page' for pmem")
Cc: <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
drivers/nvdimm/dax_devs.c | 2 +-
drivers/nvdimm/pfn.h | 1 +
drivers/nvdimm/pfn_devs.c | 18 +++++++++++++++---
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/nvdimm/dax_devs.c b/drivers/nvdimm/dax_devs.c
index 0453f49dc708..326f02ffca81 100644
--- a/drivers/nvdimm/dax_devs.c
+++ b/drivers/nvdimm/dax_devs.c
@@ -126,7 +126,7 @@ int nd_dax_probe(struct device *dev, struct nd_namespace_common *ndns)
nvdimm_bus_unlock(&ndns->dev);
if (!dax_dev)
return -ENOMEM;
- pfn_sb = devm_kzalloc(dev, sizeof(*pfn_sb), GFP_KERNEL);
+ pfn_sb = devm_kmalloc(dev, sizeof(*pfn_sb), GFP_KERNEL);
nd_pfn->pfn_sb = pfn_sb;
rc = nd_pfn_validate(nd_pfn, DAX_SIG);
dev_dbg(dev, "dax: %s\n", rc == 0 ? dev_name(dax_dev) : "<none>");
diff --git a/drivers/nvdimm/pfn.h b/drivers/nvdimm/pfn.h
index dde9853453d3..e901e3a3b04c 100644
--- a/drivers/nvdimm/pfn.h
+++ b/drivers/nvdimm/pfn.h
@@ -36,6 +36,7 @@ struct nd_pfn_sb {
__le32 end_trunc;
/* minor-version-2 record the base alignment of the mapping */
__le32 align;
+ /* minor-version-3 guarantee the padding and flags are zero */
u8 padding[4000];
__le64 checksum;
};
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 01f40672507f..a2406253eb70 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -420,6 +420,15 @@ static int nd_pfn_clear_memmap_errors(struct nd_pfn *nd_pfn)
return 0;
}
+/**
+ * nd_pfn_validate - read and validate info-block
+ * @nd_pfn: fsdax namespace runtime state / properties
+ * @sig: 'devdax' or 'fsdax' signature
+ *
+ * Upon return the info-block buffer contents (->pfn_sb) are
+ * indeterminate when validation fails, and a coherent info-block
+ * otherwise.
+ */
int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
{
u64 checksum, offset;
@@ -565,7 +574,7 @@ int nd_pfn_probe(struct device *dev, struct nd_namespace_common *ndns)
nvdimm_bus_unlock(&ndns->dev);
if (!pfn_dev)
return -ENOMEM;
- pfn_sb = devm_kzalloc(dev, sizeof(*pfn_sb), GFP_KERNEL);
+ pfn_sb = devm_kmalloc(dev, sizeof(*pfn_sb), GFP_KERNEL);
nd_pfn = to_nd_pfn(pfn_dev);
nd_pfn->pfn_sb = pfn_sb;
rc = nd_pfn_validate(nd_pfn, PFN_SIG);
@@ -702,7 +711,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
u64 checksum;
int rc;
- pfn_sb = devm_kzalloc(&nd_pfn->dev, sizeof(*pfn_sb), GFP_KERNEL);
+ pfn_sb = devm_kmalloc(&nd_pfn->dev, sizeof(*pfn_sb), GFP_KERNEL);
if (!pfn_sb)
return -ENOMEM;
@@ -711,11 +720,14 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
sig = DAX_SIG;
else
sig = PFN_SIG;
+
rc = nd_pfn_validate(nd_pfn, sig);
if (rc != -ENODEV)
return rc;
/* no info block, do init */;
+ memset(pfn_sb, 0, sizeof(*pfn_sb));
+
nd_region = to_nd_region(nd_pfn->dev.parent);
if (nd_region->ro) {
dev_info(&nd_pfn->dev,
@@ -768,7 +780,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
memcpy(pfn_sb->uuid, nd_pfn->uuid, 16);
memcpy(pfn_sb->parent_uuid, nd_dev_to_uuid(&ndns->dev), 16);
pfn_sb->version_major = cpu_to_le16(1);
- pfn_sb->version_minor = cpu_to_le16(2);
+ pfn_sb->version_minor = cpu_to_le16(3);
pfn_sb->start_pad = cpu_to_le32(start_pad);
pfn_sb->end_trunc = cpu_to_le32(end_trunc);
pfn_sb->align = cpu_to_le32(nd_pfn->align);
Now that the mm core supports section-unaligned hotplug of ZONE_DEVICE
memory, we no longer need to add padding at pfn/dax device creation
time. The kernel will still honor padding established by older kernels.
Reported-by: Jeff Moyer <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
drivers/nvdimm/pfn.h | 14 --------
drivers/nvdimm/pfn_devs.c | 77 ++++++++-------------------------------------
include/linux/mmzone.h | 3 ++
3 files changed, 16 insertions(+), 78 deletions(-)
diff --git a/drivers/nvdimm/pfn.h b/drivers/nvdimm/pfn.h
index e901e3a3b04c..cc042a98758f 100644
--- a/drivers/nvdimm/pfn.h
+++ b/drivers/nvdimm/pfn.h
@@ -41,18 +41,4 @@ struct nd_pfn_sb {
__le64 checksum;
};
-#ifdef CONFIG_SPARSEMEM
-#define PFN_SECTION_ALIGN_DOWN(x) SECTION_ALIGN_DOWN(x)
-#define PFN_SECTION_ALIGN_UP(x) SECTION_ALIGN_UP(x)
-#else
-/*
- * In this case ZONE_DEVICE=n and we will disable 'pfn' device support,
- * but we still want pmem to compile.
- */
-#define PFN_SECTION_ALIGN_DOWN(x) (x)
-#define PFN_SECTION_ALIGN_UP(x) (x)
-#endif
-
-#define PHYS_SECTION_ALIGN_DOWN(x) PFN_PHYS(PFN_SECTION_ALIGN_DOWN(PHYS_PFN(x)))
-#define PHYS_SECTION_ALIGN_UP(x) PFN_PHYS(PFN_SECTION_ALIGN_UP(PHYS_PFN(x)))
#endif /* __NVDIMM_PFN_H */
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index a2406253eb70..7f54374b082f 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -595,14 +595,14 @@ static u32 info_block_reserve(void)
}
/*
- * We hotplug memory at section granularity, pad the reserved area from
- * the previous section base to the namespace base address.
+ * We hotplug memory at sub-section granularity, pad the reserved area
+ * from the previous section base to the namespace base address.
*/
static unsigned long init_altmap_base(resource_size_t base)
{
unsigned long base_pfn = PHYS_PFN(base);
- return PFN_SECTION_ALIGN_DOWN(base_pfn);
+ return SUBSECTION_ALIGN_DOWN(base_pfn);
}
static unsigned long init_altmap_reserve(resource_size_t base)
@@ -610,7 +610,7 @@ static unsigned long init_altmap_reserve(resource_size_t base)
unsigned long reserve = info_block_reserve() >> PAGE_SHIFT;
unsigned long base_pfn = PHYS_PFN(base);
- reserve += base_pfn - PFN_SECTION_ALIGN_DOWN(base_pfn);
+ reserve += base_pfn - SUBSECTION_ALIGN_DOWN(base_pfn);
return reserve;
}
@@ -641,8 +641,7 @@ static int __nvdimm_setup_pfn(struct nd_pfn *nd_pfn, struct dev_pagemap *pgmap)
nd_pfn->npfns = le64_to_cpu(pfn_sb->npfns);
pgmap->altmap_valid = false;
} else if (nd_pfn->mode == PFN_MODE_PMEM) {
- nd_pfn->npfns = PFN_SECTION_ALIGN_UP((resource_size(res)
- - offset) / PAGE_SIZE);
+ nd_pfn->npfns = PHYS_PFN((resource_size(res) - offset));
if (le64_to_cpu(nd_pfn->pfn_sb->npfns) > nd_pfn->npfns)
dev_info(&nd_pfn->dev,
"number of pfns truncated from %lld to %ld\n",
@@ -658,54 +657,14 @@ static int __nvdimm_setup_pfn(struct nd_pfn *nd_pfn, struct dev_pagemap *pgmap)
return 0;
}
-static u64 phys_pmem_align_down(struct nd_pfn *nd_pfn, u64 phys)
-{
- return min_t(u64, PHYS_SECTION_ALIGN_DOWN(phys),
- ALIGN_DOWN(phys, nd_pfn->align));
-}
-
-/*
- * Check if pmem collides with 'System RAM', or other regions when
- * section aligned. Trim it accordingly.
- */
-static void trim_pfn_device(struct nd_pfn *nd_pfn, u32 *start_pad, u32 *end_trunc)
-{
- struct nd_namespace_common *ndns = nd_pfn->ndns;
- struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
- struct nd_region *nd_region = to_nd_region(nd_pfn->dev.parent);
- const resource_size_t start = nsio->res.start;
- const resource_size_t end = start + resource_size(&nsio->res);
- resource_size_t adjust, size;
-
- *start_pad = 0;
- *end_trunc = 0;
-
- adjust = start - PHYS_SECTION_ALIGN_DOWN(start);
- size = resource_size(&nsio->res) + adjust;
- if (region_intersects(start - adjust, size, IORESOURCE_SYSTEM_RAM,
- IORES_DESC_NONE) == REGION_MIXED
- || nd_region_conflict(nd_region, start - adjust, size))
- *start_pad = PHYS_SECTION_ALIGN_UP(start) - start;
-
- /* Now check that end of the range does not collide. */
- adjust = PHYS_SECTION_ALIGN_UP(end) - end;
- size = resource_size(&nsio->res) + adjust;
- if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM,
- IORES_DESC_NONE) == REGION_MIXED
- || !IS_ALIGNED(end, nd_pfn->align)
- || nd_region_conflict(nd_region, start, size))
- *end_trunc = end - phys_pmem_align_down(nd_pfn, end);
-}
-
static int nd_pfn_init(struct nd_pfn *nd_pfn)
{
struct nd_namespace_common *ndns = nd_pfn->ndns;
struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
- u32 start_pad, end_trunc, reserve = info_block_reserve();
resource_size_t start, size;
struct nd_region *nd_region;
+ unsigned long npfns, align;
struct nd_pfn_sb *pfn_sb;
- unsigned long npfns;
phys_addr_t offset;
const char *sig;
u64 checksum;
@@ -736,43 +695,35 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
return -ENXIO;
}
- memset(pfn_sb, 0, sizeof(*pfn_sb));
-
- trim_pfn_device(nd_pfn, &start_pad, &end_trunc);
- if (start_pad + end_trunc)
- dev_info(&nd_pfn->dev, "%s alignment collision, truncate %d bytes\n",
- dev_name(&ndns->dev), start_pad + end_trunc);
-
/*
* Note, we use 64 here for the standard size of struct page,
* debugging options may cause it to be larger in which case the
* implementation will limit the pfns advertised through
* ->direct_access() to those that are included in the memmap.
*/
- start = nsio->res.start + start_pad;
+ start = nsio->res.start;
size = resource_size(&nsio->res);
- npfns = PFN_SECTION_ALIGN_UP((size - start_pad - end_trunc - reserve)
- / PAGE_SIZE);
+ npfns = PHYS_PFN(size - SZ_8K);
+ align = max(nd_pfn->align, (1UL << SUBSECTION_SHIFT));
if (nd_pfn->mode == PFN_MODE_PMEM) {
/*
* The altmap should be padded out to the block size used
* when populating the vmemmap. This *should* be equal to
* PMD_SIZE for most architectures.
*/
- offset = ALIGN(start + reserve + 64 * npfns,
- max(nd_pfn->align, PMD_SIZE)) - start;
+ offset = ALIGN(start + SZ_8K + 64 * npfns, align) - start;
} else if (nd_pfn->mode == PFN_MODE_RAM)
- offset = ALIGN(start + reserve, nd_pfn->align) - start;
+ offset = ALIGN(start + SZ_8K, align) - start;
else
return -ENXIO;
- if (offset + start_pad + end_trunc >= size) {
+ if (offset >= size) {
dev_err(&nd_pfn->dev, "%s unable to satisfy requested alignment\n",
dev_name(&ndns->dev));
return -ENXIO;
}
- npfns = (size - offset - start_pad - end_trunc) / SZ_4K;
+ npfns = PHYS_PFN(size - offset);
pfn_sb->mode = cpu_to_le32(nd_pfn->mode);
pfn_sb->dataoff = cpu_to_le64(offset);
pfn_sb->npfns = cpu_to_le64(npfns);
@@ -781,8 +732,6 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
memcpy(pfn_sb->parent_uuid, nd_dev_to_uuid(&ndns->dev), 16);
pfn_sb->version_major = cpu_to_le16(1);
pfn_sb->version_minor = cpu_to_le16(3);
- pfn_sb->start_pad = cpu_to_le32(start_pad);
- pfn_sb->end_trunc = cpu_to_le32(end_trunc);
pfn_sb->align = cpu_to_le32(nd_pfn->align);
checksum = nd_sb_checksum((struct nd_gen_sb *) pfn_sb);
pfn_sb->checksum = cpu_to_le64(checksum);
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 49e7fb452dfd..15e07f007ba2 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1181,6 +1181,9 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
#define SUBSECTIONS_PER_SECTION (1UL << (SECTION_SIZE_BITS - SUBSECTION_SHIFT))
#endif
+#define SUBSECTION_ALIGN_UP(pfn) ALIGN((pfn), PAGES_PER_SUBSECTION)
+#define SUBSECTION_ALIGN_DOWN(pfn) ((pfn) & PAGE_SUBSECTION_MASK)
+
struct mem_section_usage {
DECLARE_BITMAP(subsection_map, SUBSECTIONS_PER_SECTION);
/* See declaration of similar field in struct zone */
On Wed, Jun 05, 2019 at 02:58:21PM -0700, Dan Williams wrote:
> Allow sub-section sized ranges to be added to the memmap.
> populate_section_memmap() takes an explict pfn range rather than
> assuming a full section, and those parameters are plumbed all the way
> through to vmmemap_populate(). There should be no sub-section usage in
> current deployments. New warnings are added to clarify which memmap
> allocation paths are sub-section capable.
>
> Cc: Michal Hocko <[email protected]>
> Cc: David Hildenbrand <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Reviewed-by: Pavel Tatashin <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
--
Oscar Salvador
SUSE L3
On Wed, Jun 05, 2019 at 02:58:37PM -0700, Dan Williams wrote:
> Prepare the memory hot-{add,remove} paths for handling sub-section
> ranges by plumbing the starting page frame and number of pages being
> handled through arch_{add,remove}_memory() to
> sparse_{add,remove}_one_section().
>
> This is simply plumbing, small cleanups, and some identifier renames. No
> intended functional changes.
>
> Cc: Michal Hocko <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Reviewed-by: Pavel Tatashin <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> include/linux/memory_hotplug.h | 5 +-
> mm/memory_hotplug.c | 114 +++++++++++++++++++++++++---------------
> mm/sparse.c | 15 ++---
> 3 files changed, 81 insertions(+), 53 deletions(-)
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 79e0add6a597..3ab0282b4fe5 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -348,9 +348,10 @@ extern int add_memory_resource(int nid, struct resource *resource);
> extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
> unsigned long nr_pages, struct vmem_altmap *altmap);
> extern bool is_memblock_offlined(struct memory_block *mem);
> -extern int sparse_add_one_section(int nid, unsigned long start_pfn,
> - struct vmem_altmap *altmap);
> +extern int sparse_add_section(int nid, unsigned long pfn,
> + unsigned long nr_pages, struct vmem_altmap *altmap);
> extern void sparse_remove_one_section(struct mem_section *ms,
> + unsigned long pfn, unsigned long nr_pages,
> unsigned long map_offset, struct vmem_altmap *altmap);
> extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
> unsigned long pnum);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 4b882c57781a..399bf78bccc5 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -252,51 +252,84 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
> }
> #endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
>
> -static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
> - struct vmem_altmap *altmap)
> +static int __meminit __add_section(int nid, unsigned long pfn,
> + unsigned long nr_pages, struct vmem_altmap *altmap)
> {
> int ret;
>
> - if (pfn_valid(phys_start_pfn))
> + if (pfn_valid(pfn))
> return -EEXIST;
>
> - ret = sparse_add_one_section(nid, phys_start_pfn, altmap);
> + ret = sparse_add_section(nid, pfn, nr_pages, altmap);
> return ret < 0 ? ret : 0;
> }
>
> +static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
> + const char *reason)
> +{
> + /*
> + * Disallow all operations smaller than a sub-section and only
> + * allow operations smaller than a section for
> + * SPARSEMEM_VMEMMAP. Note that check_hotplug_memory_range()
> + * enforces a larger memory_block_size_bytes() granularity for
> + * memory that will be marked online, so this check should only
> + * fire for direct arch_{add,remove}_memory() users outside of
> + * add_memory_resource().
> + */
> + unsigned long min_align;
> +
> + if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
> + min_align = PAGES_PER_SUBSECTION;
> + else
> + min_align = PAGES_PER_SECTION;
> + if (!IS_ALIGNED(pfn, min_align)
> + || !IS_ALIGNED(nr_pages, min_align)) {
> + WARN(1, "Misaligned __%s_pages start: %#lx end: #%lx\n",
> + reason, pfn, pfn + nr_pages - 1);
> + return -EINVAL;
> + }
> + return 0;
> +}
This caught my eye.
Back in patch#4 "Convert kmalloc_section_memmap() to populate_section_memmap()",
you placed a mis-usage check for !CONFIG_SPARSEMEM_VMEMMAP in
populate_section_memmap().
populate_section_memmap() gets called from sparse_add_one_section(), which means
that we should have passed this check, otherwise we cannot go further and call
__add_section().
So, unless I am missing something it seems to me that the check from patch#4 could go?
And I think the same applies to depopulate_section_memmap()?
Besides that, it looks good to me:
Reviewed-by: Oscar Salvador <[email protected]>
--
Oscar Salvador
SUSE L3
On Wed, Jun 05, 2019 at 02:57:54PM -0700, Dan Williams wrote:
> Towards enabling memory hotplug to track partial population of a
> section, introduce 'struct mem_section_usage'.
>
> A pointer to a 'struct mem_section_usage' instance replaces the existing
> pointer to a 'pageblock_flags' bitmap. Effectively it adds one more
> 'unsigned long' beyond the 'pageblock_flags' (usemap) allocation to
> house a new 'subsection_map' bitmap. The new bitmap enables the memory
> hot{plug,remove} implementation to act on incremental sub-divisions of a
> section.
>
> The default SUBSECTION_SHIFT is chosen to keep the 'subsection_map' no
> larger than a single 'unsigned long' on the major architectures.
> Alternatively an architecture can define ARCH_SUBSECTION_SHIFT to
> override the default PMD_SHIFT. Note that PowerPC needs to use
> ARCH_SUBSECTION_SHIFT to workaround PMD_SHIFT being a non-constant
> expression on PowerPC.
>
> The primary motivation for this functionality is to support platforms
> that mix "System RAM" and "Persistent Memory" within a single section,
> or multiple PMEM ranges with different mapping lifetimes within a single
> section. The section restriction for hotplug has caused an ongoing saga
> of hacks and bugs for devm_memremap_pages() users.
>
> Beyond the fixups to teach existing paths how to retrieve the 'usemap'
> from a section, and updates to usemap allocation path, there are no
> expected behavior changes.
>
> Cc: Michal Hocko <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Cc: Pavel Tatashin <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
--
Oscar Salvador
SUSE L3
On Thu, Jun 6, 2019 at 10:21 AM Oscar Salvador <[email protected]> wrote:
>
> On Wed, Jun 05, 2019 at 02:58:37PM -0700, Dan Williams wrote:
> > Prepare the memory hot-{add,remove} paths for handling sub-section
> > ranges by plumbing the starting page frame and number of pages being
> > handled through arch_{add,remove}_memory() to
> > sparse_{add,remove}_one_section().
> >
> > This is simply plumbing, small cleanups, and some identifier renames. No
> > intended functional changes.
> >
> > Cc: Michal Hocko <[email protected]>
> > Cc: Vlastimil Babka <[email protected]>
> > Cc: Logan Gunthorpe <[email protected]>
> > Cc: Oscar Salvador <[email protected]>
> > Reviewed-by: Pavel Tatashin <[email protected]>
> > Signed-off-by: Dan Williams <[email protected]>
> > ---
> > include/linux/memory_hotplug.h | 5 +-
> > mm/memory_hotplug.c | 114 +++++++++++++++++++++++++---------------
> > mm/sparse.c | 15 ++---
> > 3 files changed, 81 insertions(+), 53 deletions(-)
> >
> > diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> > index 79e0add6a597..3ab0282b4fe5 100644
> > --- a/include/linux/memory_hotplug.h
> > +++ b/include/linux/memory_hotplug.h
> > @@ -348,9 +348,10 @@ extern int add_memory_resource(int nid, struct resource *resource);
> > extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
> > unsigned long nr_pages, struct vmem_altmap *altmap);
> > extern bool is_memblock_offlined(struct memory_block *mem);
> > -extern int sparse_add_one_section(int nid, unsigned long start_pfn,
> > - struct vmem_altmap *altmap);
> > +extern int sparse_add_section(int nid, unsigned long pfn,
> > + unsigned long nr_pages, struct vmem_altmap *altmap);
> > extern void sparse_remove_one_section(struct mem_section *ms,
> > + unsigned long pfn, unsigned long nr_pages,
> > unsigned long map_offset, struct vmem_altmap *altmap);
> > extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
> > unsigned long pnum);
> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index 4b882c57781a..399bf78bccc5 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -252,51 +252,84 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
> > }
> > #endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
> >
> > -static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
> > - struct vmem_altmap *altmap)
> > +static int __meminit __add_section(int nid, unsigned long pfn,
> > + unsigned long nr_pages, struct vmem_altmap *altmap)
> > {
> > int ret;
> >
> > - if (pfn_valid(phys_start_pfn))
> > + if (pfn_valid(pfn))
> > return -EEXIST;
> >
> > - ret = sparse_add_one_section(nid, phys_start_pfn, altmap);
> > + ret = sparse_add_section(nid, pfn, nr_pages, altmap);
> > return ret < 0 ? ret : 0;
> > }
> >
> > +static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
> > + const char *reason)
> > +{
> > + /*
> > + * Disallow all operations smaller than a sub-section and only
> > + * allow operations smaller than a section for
> > + * SPARSEMEM_VMEMMAP. Note that check_hotplug_memory_range()
> > + * enforces a larger memory_block_size_bytes() granularity for
> > + * memory that will be marked online, so this check should only
> > + * fire for direct arch_{add,remove}_memory() users outside of
> > + * add_memory_resource().
> > + */
> > + unsigned long min_align;
> > +
> > + if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
> > + min_align = PAGES_PER_SUBSECTION;
> > + else
> > + min_align = PAGES_PER_SECTION;
> > + if (!IS_ALIGNED(pfn, min_align)
> > + || !IS_ALIGNED(nr_pages, min_align)) {
> > + WARN(1, "Misaligned __%s_pages start: %#lx end: #%lx\n",
> > + reason, pfn, pfn + nr_pages - 1);
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
>
>
> This caught my eye.
> Back in patch#4 "Convert kmalloc_section_memmap() to populate_section_memmap()",
> you placed a mis-usage check for !CONFIG_SPARSEMEM_VMEMMAP in
> populate_section_memmap().
>
> populate_section_memmap() gets called from sparse_add_one_section(), which means
> that we should have passed this check, otherwise we cannot go further and call
> __add_section().
>
> So, unless I am missing something it seems to me that the check from patch#4 could go?
> And I think the same applies to depopulate_section_memmap()?
Yes, good catch, I can kill those extra checks in favor of this one.
> Besides that, it looks good to me:
Thanks Oscar!
>
> Reviewed-by: Oscar Salvador <[email protected]>
>
> --
> Oscar Salvador
> SUSE L3
On Wed, Jun 05, 2019 at 02:57:59PM -0700, Dan Williams wrote:
> Prepare for hot{plug,remove} of sub-ranges of a section by tracking a
> sub-section active bitmask, each bit representing a PMD_SIZE span of the
> architecture's memory hotplug section size.
>
> The implications of a partially populated section is that pfn_valid()
> needs to go beyond a valid_section() check and read the sub-section
> active ranges from the bitmask. The expectation is that the bitmask
> (subsection_map) fits in the same cacheline as the valid_section() data,
> so the incremental performance overhead to pfn_valid() should be
> negligible.
>
> Cc: Michal Hocko <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Cc: Pavel Tatashin <[email protected]>
> Tested-by: Jane Chu <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
--
Oscar Salvador
SUSE L3
On Wed, 05 Jun 2019 14:58:58 -0700 Dan Williams <[email protected]> wrote:
> At namespace creation time there is the potential for the "expected to
> be zero" fields of a 'pfn' info-block to be filled with indeterminate
> data. While the kernel buffer is zeroed on allocation it is immediately
> overwritten by nd_pfn_validate() filling it with the current contents of
> the on-media info-block location. For fields like, 'flags' and the
> 'padding' it potentially means that future implementations can not rely
> on those fields being zero.
>
> In preparation to stop using the 'start_pad' and 'end_trunc' fields for
> section alignment, arrange for fields that are not explicitly
> initialized to be guaranteed zero. Bump the minor version to indicate it
> is safe to assume the 'padding' and 'flags' are zero. Otherwise, this
> corruption is expected to benign since all other critical fields are
> explicitly initialized.
>
> Fixes: 32ab0a3f5170 ("libnvdimm, pmem: 'struct page' for pmem")
> Cc: <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
The cc:stable in [11/12] seems odd. Is this independent of the other
patches? If so, shouldn't it be a standalone thing which can be
prioritized?
On Thu, Jun 6, 2019 at 2:46 PM Andrew Morton <[email protected]> wrote:
>
> On Wed, 05 Jun 2019 14:58:58 -0700 Dan Williams <[email protected]> wrote:
>
> > At namespace creation time there is the potential for the "expected to
> > be zero" fields of a 'pfn' info-block to be filled with indeterminate
> > data. While the kernel buffer is zeroed on allocation it is immediately
> > overwritten by nd_pfn_validate() filling it with the current contents of
> > the on-media info-block location. For fields like, 'flags' and the
> > 'padding' it potentially means that future implementations can not rely
> > on those fields being zero.
> >
> > In preparation to stop using the 'start_pad' and 'end_trunc' fields for
> > section alignment, arrange for fields that are not explicitly
> > initialized to be guaranteed zero. Bump the minor version to indicate it
> > is safe to assume the 'padding' and 'flags' are zero. Otherwise, this
> > corruption is expected to benign since all other critical fields are
> > explicitly initialized.
> >
> > Fixes: 32ab0a3f5170 ("libnvdimm, pmem: 'struct page' for pmem")
> > Cc: <[email protected]>
> > Signed-off-by: Dan Williams <[email protected]>
>
> The cc:stable in [11/12] seems odd. Is this independent of the other
> patches? If so, shouldn't it be a standalone thing which can be
> prioritized?
>
The cc: stable is about spreading this new policy to as many kernels
as possible not fixing an issue in those kernels. It's not until patch
12 "libnvdimm/pfn: Stop padding pmem namespaces to section alignment"
as all previous kernel do initialize all fields.
I'd be ok to drop that cc: stable, my concern is distros that somehow
pickup and backport patch 12 and miss patch 11.
On Wed, Jun 05, 2019 at 02:58:42PM -0700, Dan Williams wrote:
> The libnvdimm sub-system has suffered a series of hacks and broken
> workarounds for the memory-hotplug implementation's awkward
> section-aligned (128MB) granularity. For example the following backtrace
> is emitted when attempting arch_add_memory() with physical address
> ranges that intersect 'System RAM' (RAM) with 'Persistent Memory' (PMEM)
> within a given section:
>
> WARNING: CPU: 0 PID: 558 at kernel/memremap.c:300 devm_memremap_pages+0x3b5/0x4c0
> devm_memremap_pages attempted on mixed region [mem 0x200000000-0x2fbffffff flags 0x200]
> [..]
> Call Trace:
> dump_stack+0x86/0xc3
> __warn+0xcb/0xf0
> warn_slowpath_fmt+0x5f/0x80
> devm_memremap_pages+0x3b5/0x4c0
> __wrap_devm_memremap_pages+0x58/0x70 [nfit_test_iomap]
> pmem_attach_disk+0x19a/0x440 [nd_pmem]
>
> Recently it was discovered that the problem goes beyond RAM vs PMEM
> collisions as some platform produce PMEM vs PMEM collisions within a
> given section. The libnvdimm workaround for that case revealed that the
> libnvdimm section-alignment-padding implementation has been broken for a
> long while. A fix for that long-standing breakage introduces as many
> problems as it solves as it would require a backward-incompatible change
> to the namespace metadata interpretation. Instead of that dubious route
> [1], address the root problem in the memory-hotplug implementation.
>
> [1]: https://lore.kernel.org/r/155000671719.348031.2347363160141119237.stgit@dwillia2-desk3.amr.corp.intel.com
> Cc: Michal Hocko <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Cc: Pavel Tatashin <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> include/linux/memory_hotplug.h | 2
> mm/memory_hotplug.c | 7 -
> mm/page_alloc.c | 2
> mm/sparse.c | 225 +++++++++++++++++++++++++++-------------
> 4 files changed, 155 insertions(+), 81 deletions(-)
>
[...]
> @@ -325,6 +332,15 @@ static void __meminit sparse_init_one_section(struct mem_section *ms,
> unsigned long pnum, struct page *mem_map,
> struct mem_section_usage *usage)
> {
> + /*
> + * Given that SPARSEMEM_VMEMMAP=y supports sub-section hotplug,
> + * ->section_mem_map can not be guaranteed to point to a full
> + * section's worth of memory. The field is only valid / used
> + * in the SPARSEMEM_VMEMMAP=n case.
> + */
> + if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
> + mem_map = NULL;
Will this be a problem when reading mem_map with the crash-tool?
I do not expect it to be, but I am not sure if crash internally tries
to read ms->section_mem_map and do some sort of translation.
And since ms->section_mem_map SECTION_HAS_MEM_MAP, it might be that it expects
a valid mem_map?
> +static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> + struct vmem_altmap *altmap)
> +{
> + DECLARE_BITMAP(map, SUBSECTIONS_PER_SECTION) = { 0 };
> + DECLARE_BITMAP(tmp, SUBSECTIONS_PER_SECTION) = { 0 };
> + struct mem_section *ms = __pfn_to_section(pfn);
> + bool early_section = is_early_section(ms);
> + struct page *memmap = NULL;
> + unsigned long *subsection_map = ms->usage
> + ? &ms->usage->subsection_map[0] : NULL;
> +
> + subsection_mask_set(map, pfn, nr_pages);
> + if (subsection_map)
> + bitmap_and(tmp, map, subsection_map, SUBSECTIONS_PER_SECTION);
> +
> + if (WARN(!subsection_map || !bitmap_equal(tmp, map, SUBSECTIONS_PER_SECTION),
> + "section already deactivated (%#lx + %ld)\n",
> + pfn, nr_pages))
> + return;
> +
> + /*
> + * There are 3 cases to handle across two configurations
> + * (SPARSEMEM_VMEMMAP={y,n}):
> + *
> + * 1/ deactivation of a partial hot-added section (only possible
> + * in the SPARSEMEM_VMEMMAP=y case).
> + * a/ section was present at memory init
> + * b/ section was hot-added post memory init
> + * 2/ deactivation of a complete hot-added section
> + * 3/ deactivation of a complete section from memory init
> + *
> + * For 1/, when subsection_map does not empty we will not be
> + * freeing the usage map, but still need to free the vmemmap
> + * range.
> + *
> + * For 2/ and 3/ the SPARSEMEM_VMEMMAP={y,n} cases are unified
> + */
> + bitmap_xor(subsection_map, map, subsection_map, SUBSECTIONS_PER_SECTION);
> + if (bitmap_empty(subsection_map, SUBSECTIONS_PER_SECTION)) {
> + unsigned long section_nr = pfn_to_section_nr(pfn);
> +
> + if (!early_section) {
> + kfree(ms->usage);
> + ms->usage = NULL;
> + }
> + memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> + ms->section_mem_map = sparse_encode_mem_map(NULL, section_nr);
> + }
> +
> + if (early_section && memmap)
> + free_map_bootmem(memmap);
> + else
> + depopulate_section_memmap(pfn, nr_pages, altmap);
> +}
> +
> +static struct page * __meminit section_activate(int nid, unsigned long pfn,
> + unsigned long nr_pages, struct vmem_altmap *altmap)
> +{
> + DECLARE_BITMAP(map, SUBSECTIONS_PER_SECTION) = { 0 };
> + struct mem_section *ms = __pfn_to_section(pfn);
> + struct mem_section_usage *usage = NULL;
> + unsigned long *subsection_map;
> + struct page *memmap;
> + int rc = 0;
> +
> + subsection_mask_set(map, pfn, nr_pages);
> +
> + if (!ms->usage) {
> + usage = kzalloc(mem_section_usage_size(), GFP_KERNEL);
> + if (!usage)
> + return ERR_PTR(-ENOMEM);
> + ms->usage = usage;
> + }
> + subsection_map = &ms->usage->subsection_map[0];
> +
> + if (bitmap_empty(map, SUBSECTIONS_PER_SECTION))
> + rc = -EINVAL;
> + else if (bitmap_intersects(map, subsection_map, SUBSECTIONS_PER_SECTION))
> + rc = -EEXIST;
> + else
> + bitmap_or(subsection_map, map, subsection_map,
> + SUBSECTIONS_PER_SECTION);
> +
> + if (rc) {
> + if (usage)
> + ms->usage = NULL;
> + kfree(usage);
> + return ERR_PTR(rc);
> + }
We should not be really looking at subsection_map stuff when running on
!CONFIG_SPARSE_VMEMMAP, right?
Would it make sense to hide the bitmap dance behind
if(IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP)) ?
Sorry for nagging here
> /**
> - * sparse_add_one_section - add a memory section
> + * sparse_add_section - add a memory section, or populate an existing one
> * @nid: The node to add section on
> * @start_pfn: start pfn of the memory range
> + * @nr_pages: number of pfns to add in the section
> * @altmap: device page map
> *
> * This is only intended for hotplug.
Below this, the return codes are specified:
---
* Return:
* * 0 - On success.
* * -EEXIST - Section has been present.
* * -ENOMEM - Out of memory.
*/
---
We can get rid of -EEXIST since we do not return that anymore.
--
Oscar Salvador
SUSE L3
On Wed, Jun 05, 2019 at 02:58:53PM -0700, Dan Williams wrote:
> Teach devm_memremap_pages() about the new sub-section capabilities of
> arch_{add,remove}_memory(). Effectively, just replace all usage of
> align_start, align_end, and align_size with res->start, res->end, and
> resource_size(res). The existing sanity check will still make sure that
> the two separate remap attempts do not collide within a sub-section (2MB
> on x86).
>
> Cc: Michal Hocko <[email protected]>
> Cc: Toshi Kani <[email protected]>
> Cc: J?r?me Glisse <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Cc: Pavel Tatashin <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
Looks good to me:
Reviewed-by: Oscar Salvador <[email protected]>
--
Oscar Salvador
SUSE L3
On Fri, Jun 7, 2019 at 1:34 AM Oscar Salvador <[email protected]> wrote:
>
> On Wed, Jun 05, 2019 at 02:58:42PM -0700, Dan Williams wrote:
> > The libnvdimm sub-system has suffered a series of hacks and broken
> > workarounds for the memory-hotplug implementation's awkward
> > section-aligned (128MB) granularity. For example the following backtrace
> > is emitted when attempting arch_add_memory() with physical address
> > ranges that intersect 'System RAM' (RAM) with 'Persistent Memory' (PMEM)
> > within a given section:
> >
> > WARNING: CPU: 0 PID: 558 at kernel/memremap.c:300 devm_memremap_pages+0x3b5/0x4c0
> > devm_memremap_pages attempted on mixed region [mem 0x200000000-0x2fbffffff flags 0x200]
> > [..]
> > Call Trace:
> > dump_stack+0x86/0xc3
> > __warn+0xcb/0xf0
> > warn_slowpath_fmt+0x5f/0x80
> > devm_memremap_pages+0x3b5/0x4c0
> > __wrap_devm_memremap_pages+0x58/0x70 [nfit_test_iomap]
> > pmem_attach_disk+0x19a/0x440 [nd_pmem]
> >
> > Recently it was discovered that the problem goes beyond RAM vs PMEM
> > collisions as some platform produce PMEM vs PMEM collisions within a
> > given section. The libnvdimm workaround for that case revealed that the
> > libnvdimm section-alignment-padding implementation has been broken for a
> > long while. A fix for that long-standing breakage introduces as many
> > problems as it solves as it would require a backward-incompatible change
> > to the namespace metadata interpretation. Instead of that dubious route
> > [1], address the root problem in the memory-hotplug implementation.
> >
> > [1]: https://lore.kernel.org/r/155000671719.348031.2347363160141119237.stgit@dwillia2-desk3.amr.corp.intel.com
> > Cc: Michal Hocko <[email protected]>
> > Cc: Vlastimil Babka <[email protected]>
> > Cc: Logan Gunthorpe <[email protected]>
> > Cc: Oscar Salvador <[email protected]>
> > Cc: Pavel Tatashin <[email protected]>
> > Signed-off-by: Dan Williams <[email protected]>
> > ---
> > include/linux/memory_hotplug.h | 2
> > mm/memory_hotplug.c | 7 -
> > mm/page_alloc.c | 2
> > mm/sparse.c | 225 +++++++++++++++++++++++++++-------------
> > 4 files changed, 155 insertions(+), 81 deletions(-)
> >
> [...]
> > @@ -325,6 +332,15 @@ static void __meminit sparse_init_one_section(struct mem_section *ms,
> > unsigned long pnum, struct page *mem_map,
> > struct mem_section_usage *usage)
> > {
> > + /*
> > + * Given that SPARSEMEM_VMEMMAP=y supports sub-section hotplug,
> > + * ->section_mem_map can not be guaranteed to point to a full
> > + * section's worth of memory. The field is only valid / used
> > + * in the SPARSEMEM_VMEMMAP=n case.
> > + */
> > + if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
> > + mem_map = NULL;
>
> Will this be a problem when reading mem_map with the crash-tool?
> I do not expect it to be, but I am not sure if crash internally tries
> to read ms->section_mem_map and do some sort of translation.
> And since ms->section_mem_map SECTION_HAS_MEM_MAP, it might be that it expects
> a valid mem_map?
I don't know, but I can't imagine it would because it's much easier to
do mem_map relative translations by simple PAGE_OFFSET arithmetic.
> > +static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> > + struct vmem_altmap *altmap)
> > +{
> > + DECLARE_BITMAP(map, SUBSECTIONS_PER_SECTION) = { 0 };
> > + DECLARE_BITMAP(tmp, SUBSECTIONS_PER_SECTION) = { 0 };
> > + struct mem_section *ms = __pfn_to_section(pfn);
> > + bool early_section = is_early_section(ms);
> > + struct page *memmap = NULL;
> > + unsigned long *subsection_map = ms->usage
> > + ? &ms->usage->subsection_map[0] : NULL;
> > +
> > + subsection_mask_set(map, pfn, nr_pages);
> > + if (subsection_map)
> > + bitmap_and(tmp, map, subsection_map, SUBSECTIONS_PER_SECTION);
> > +
> > + if (WARN(!subsection_map || !bitmap_equal(tmp, map, SUBSECTIONS_PER_SECTION),
> > + "section already deactivated (%#lx + %ld)\n",
> > + pfn, nr_pages))
> > + return;
> > +
> > + /*
> > + * There are 3 cases to handle across two configurations
> > + * (SPARSEMEM_VMEMMAP={y,n}):
> > + *
> > + * 1/ deactivation of a partial hot-added section (only possible
> > + * in the SPARSEMEM_VMEMMAP=y case).
> > + * a/ section was present at memory init
> > + * b/ section was hot-added post memory init
> > + * 2/ deactivation of a complete hot-added section
> > + * 3/ deactivation of a complete section from memory init
> > + *
> > + * For 1/, when subsection_map does not empty we will not be
> > + * freeing the usage map, but still need to free the vmemmap
> > + * range.
> > + *
> > + * For 2/ and 3/ the SPARSEMEM_VMEMMAP={y,n} cases are unified
> > + */
> > + bitmap_xor(subsection_map, map, subsection_map, SUBSECTIONS_PER_SECTION);
> > + if (bitmap_empty(subsection_map, SUBSECTIONS_PER_SECTION)) {
> > + unsigned long section_nr = pfn_to_section_nr(pfn);
> > +
> > + if (!early_section) {
> > + kfree(ms->usage);
> > + ms->usage = NULL;
> > + }
> > + memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> > + ms->section_mem_map = sparse_encode_mem_map(NULL, section_nr);
> > + }
> > +
> > + if (early_section && memmap)
> > + free_map_bootmem(memmap);
> > + else
> > + depopulate_section_memmap(pfn, nr_pages, altmap);
> > +}
> > +
> > +static struct page * __meminit section_activate(int nid, unsigned long pfn,
> > + unsigned long nr_pages, struct vmem_altmap *altmap)
> > +{
> > + DECLARE_BITMAP(map, SUBSECTIONS_PER_SECTION) = { 0 };
> > + struct mem_section *ms = __pfn_to_section(pfn);
> > + struct mem_section_usage *usage = NULL;
> > + unsigned long *subsection_map;
> > + struct page *memmap;
> > + int rc = 0;
> > +
> > + subsection_mask_set(map, pfn, nr_pages);
> > +
> > + if (!ms->usage) {
> > + usage = kzalloc(mem_section_usage_size(), GFP_KERNEL);
> > + if (!usage)
> > + return ERR_PTR(-ENOMEM);
> > + ms->usage = usage;
> > + }
> > + subsection_map = &ms->usage->subsection_map[0];
> > +
> > + if (bitmap_empty(map, SUBSECTIONS_PER_SECTION))
> > + rc = -EINVAL;
> > + else if (bitmap_intersects(map, subsection_map, SUBSECTIONS_PER_SECTION))
> > + rc = -EEXIST;
> > + else
> > + bitmap_or(subsection_map, map, subsection_map,
> > + SUBSECTIONS_PER_SECTION);
> > +
> > + if (rc) {
> > + if (usage)
> > + ms->usage = NULL;
> > + kfree(usage);
> > + return ERR_PTR(rc);
> > + }
>
> We should not be really looking at subsection_map stuff when running on
> !CONFIG_SPARSE_VMEMMAP, right?
> Would it make sense to hide the bitmap dance behind
>
> if(IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP)) ?
>
> Sorry for nagging here
No worries, its a valid question. The bitmap dance is still valid it
will just happen on section boundaries instead of subsection. If
anything breaks that's beneficial additional testing that we got from
the SPARSEMEM sub-case for the SPARSEMEM_VMEMMAP superset-case. That's
the gain for keeping them unified, what's the practical gain from
hiding this bit manipulation from the SPARSEMEM case?
>
> > /**
> > - * sparse_add_one_section - add a memory section
> > + * sparse_add_section - add a memory section, or populate an existing one
> > * @nid: The node to add section on
> > * @start_pfn: start pfn of the memory range
> > + * @nr_pages: number of pfns to add in the section
> > * @altmap: device page map
> > *
> > * This is only intended for hotplug.
>
> Below this, the return codes are specified:
>
> ---
> * Return:
> * * 0 - On success.
> * * -EEXIST - Section has been present.
> * * -ENOMEM - Out of memory.
> */
> ---
>
> We can get rid of -EEXIST since we do not return that anymore.
Good catch.
On Thu, 6 Jun 2019 15:06:26 -0700 Dan Williams <[email protected]> wrote:
> On Thu, Jun 6, 2019 at 2:46 PM Andrew Morton <[email protected]> wrote:
> >
> > On Wed, 05 Jun 2019 14:58:58 -0700 Dan Williams <[email protected]> wrote:
> >
> > > At namespace creation time there is the potential for the "expected to
> > > be zero" fields of a 'pfn' info-block to be filled with indeterminate
> > > data. While the kernel buffer is zeroed on allocation it is immediately
> > > overwritten by nd_pfn_validate() filling it with the current contents of
> > > the on-media info-block location. For fields like, 'flags' and the
> > > 'padding' it potentially means that future implementations can not rely
> > > on those fields being zero.
> > >
> > > In preparation to stop using the 'start_pad' and 'end_trunc' fields for
> > > section alignment, arrange for fields that are not explicitly
> > > initialized to be guaranteed zero. Bump the minor version to indicate it
> > > is safe to assume the 'padding' and 'flags' are zero. Otherwise, this
> > > corruption is expected to benign since all other critical fields are
> > > explicitly initialized.
> > >
> > > Fixes: 32ab0a3f5170 ("libnvdimm, pmem: 'struct page' for pmem")
> > > Cc: <[email protected]>
> > > Signed-off-by: Dan Williams <[email protected]>
> >
> > The cc:stable in [11/12] seems odd. Is this independent of the other
> > patches? If so, shouldn't it be a standalone thing which can be
> > prioritized?
> >
>
> The cc: stable is about spreading this new policy to as many kernels
> as possible not fixing an issue in those kernels. It's not until patch
> 12 "libnvdimm/pfn: Stop padding pmem namespaces to section alignment"
> as all previous kernel do initialize all fields.
>
> I'd be ok to drop that cc: stable, my concern is distros that somehow
> pickup and backport patch 12 and miss patch 11.
Could you please propose a changelog paragraph which explains all this
to those who will be considering this patch for backports?
On Fri, Jun 7, 2019 at 12:54 PM Andrew Morton <[email protected]> wrote:
>
> On Thu, 6 Jun 2019 15:06:26 -0700 Dan Williams <[email protected]> wrote:
>
> > On Thu, Jun 6, 2019 at 2:46 PM Andrew Morton <[email protected]> wrote:
> > >
> > > On Wed, 05 Jun 2019 14:58:58 -0700 Dan Williams <[email protected]> wrote:
> > >
> > > > At namespace creation time there is the potential for the "expected to
> > > > be zero" fields of a 'pfn' info-block to be filled with indeterminate
> > > > data. While the kernel buffer is zeroed on allocation it is immediately
> > > > overwritten by nd_pfn_validate() filling it with the current contents of
> > > > the on-media info-block location. For fields like, 'flags' and the
> > > > 'padding' it potentially means that future implementations can not rely
> > > > on those fields being zero.
> > > >
> > > > In preparation to stop using the 'start_pad' and 'end_trunc' fields for
> > > > section alignment, arrange for fields that are not explicitly
> > > > initialized to be guaranteed zero. Bump the minor version to indicate it
> > > > is safe to assume the 'padding' and 'flags' are zero. Otherwise, this
> > > > corruption is expected to benign since all other critical fields are
> > > > explicitly initialized.
> > > >
> > > > Fixes: 32ab0a3f5170 ("libnvdimm, pmem: 'struct page' for pmem")
> > > > Cc: <[email protected]>
> > > > Signed-off-by: Dan Williams <[email protected]>
> > >
> > > The cc:stable in [11/12] seems odd. Is this independent of the other
> > > patches? If so, shouldn't it be a standalone thing which can be
> > > prioritized?
> > >
> >
> > The cc: stable is about spreading this new policy to as many kernels
> > as possible not fixing an issue in those kernels. It's not until patch
> > 12 "libnvdimm/pfn: Stop padding pmem namespaces to section alignment"
> > as all previous kernel do initialize all fields.
> >
> > I'd be ok to drop that cc: stable, my concern is distros that somehow
> > pickup and backport patch 12 and miss patch 11.
>
> Could you please propose a changelog paragraph which explains all this
> to those who will be considering this patch for backports?
>
Will do. I'll resend the series with this and the fixups proposed by Oscar.
On Fri, 2019-06-07 at 08:38 -0700, Dan Williams wrote:
> I don't know, but I can't imagine it would because it's much easier
> to
> do mem_map relative translations by simple PAGE_OFFSET arithmetic.
Yeah, I guess so.
> No worries, its a valid question. The bitmap dance is still valid it
> will just happen on section boundaries instead of subsection. If
> anything breaks that's beneficial additional testing that we got from
> the SPARSEMEM sub-case for the SPARSEMEM_VMEMMAP superset-case.
> That's
> the gain for keeping them unified, what's the practical gain from
> hiding this bit manipulation from the SPARSEMEM case?
It is just that I thought that we might benefit from not doing extra
work if not needed (bitmap dance) in SPARSEMEM case.
But given that 1) hot-add/hot-remove paths are not hot paths, it does
not really matter 2) and that having all cases unified in one function
make sense too, spreading the work in more functions might be sub-
optimal.
I guess I could justfiy it in case both activate/deactive functions
would look convulated, but it is not the case here.
I just took another look to check that I did not miss anything.
It looks quite nice and compact IMO:
Reviewed-by: Oscar Salvador <[email protected]>
--
Oscar Salvador
SUSE L3
Dan Williams <[email protected]> writes:
> At namespace creation time there is the potential for the "expected to
> be zero" fields of a 'pfn' info-block to be filled with indeterminate
> data. While the kernel buffer is zeroed on allocation it is immediately
> overwritten by nd_pfn_validate() filling it with the current contents of
> the on-media info-block location. For fields like, 'flags' and the
> 'padding' it potentially means that future implementations can not rely
> on those fields being zero.
>
> In preparation to stop using the 'start_pad' and 'end_trunc' fields for
> section alignment, arrange for fields that are not explicitly
> initialized to be guaranteed zero. Bump the minor version to indicate it
> is safe to assume the 'padding' and 'flags' are zero. Otherwise, this
> corruption is expected to benign since all other critical fields are
> explicitly initialized.
>
> Fixes: 32ab0a3f5170 ("libnvdimm, pmem: 'struct page' for pmem")
> Cc: <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> drivers/nvdimm/dax_devs.c | 2 +-
> drivers/nvdimm/pfn.h | 1 +
> drivers/nvdimm/pfn_devs.c | 18 +++++++++++++++---
> 3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvdimm/dax_devs.c b/drivers/nvdimm/dax_devs.c
> index 0453f49dc708..326f02ffca81 100644
> --- a/drivers/nvdimm/dax_devs.c
> +++ b/drivers/nvdimm/dax_devs.c
> @@ -126,7 +126,7 @@ int nd_dax_probe(struct device *dev, struct nd_namespace_common *ndns)
> nvdimm_bus_unlock(&ndns->dev);
> if (!dax_dev)
> return -ENOMEM;
> - pfn_sb = devm_kzalloc(dev, sizeof(*pfn_sb), GFP_KERNEL);
> + pfn_sb = devm_kmalloc(dev, sizeof(*pfn_sb), GFP_KERNEL);
> nd_pfn->pfn_sb = pfn_sb;
> rc = nd_pfn_validate(nd_pfn, DAX_SIG);
> dev_dbg(dev, "dax: %s\n", rc == 0 ? dev_name(dax_dev) : "<none>");
> diff --git a/drivers/nvdimm/pfn.h b/drivers/nvdimm/pfn.h
> index dde9853453d3..e901e3a3b04c 100644
> --- a/drivers/nvdimm/pfn.h
> +++ b/drivers/nvdimm/pfn.h
> @@ -36,6 +36,7 @@ struct nd_pfn_sb {
> __le32 end_trunc;
> /* minor-version-2 record the base alignment of the mapping */
> __le32 align;
> + /* minor-version-3 guarantee the padding and flags are zero */
> u8 padding[4000];
> __le64 checksum;
> };
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index 01f40672507f..a2406253eb70 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -420,6 +420,15 @@ static int nd_pfn_clear_memmap_errors(struct nd_pfn *nd_pfn)
> return 0;
> }
>
> +/**
> + * nd_pfn_validate - read and validate info-block
> + * @nd_pfn: fsdax namespace runtime state / properties
> + * @sig: 'devdax' or 'fsdax' signature
> + *
> + * Upon return the info-block buffer contents (->pfn_sb) are
> + * indeterminate when validation fails, and a coherent info-block
> + * otherwise.
> + */
> int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
> {
> u64 checksum, offset;
> @@ -565,7 +574,7 @@ int nd_pfn_probe(struct device *dev, struct nd_namespace_common *ndns)
> nvdimm_bus_unlock(&ndns->dev);
> if (!pfn_dev)
> return -ENOMEM;
> - pfn_sb = devm_kzalloc(dev, sizeof(*pfn_sb), GFP_KERNEL);
> + pfn_sb = devm_kmalloc(dev, sizeof(*pfn_sb), GFP_KERNEL);
> nd_pfn = to_nd_pfn(pfn_dev);
> nd_pfn->pfn_sb = pfn_sb;
> rc = nd_pfn_validate(nd_pfn, PFN_SIG);
> @@ -702,7 +711,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
> u64 checksum;
> int rc;
>
> - pfn_sb = devm_kzalloc(&nd_pfn->dev, sizeof(*pfn_sb), GFP_KERNEL);
> + pfn_sb = devm_kmalloc(&nd_pfn->dev, sizeof(*pfn_sb), GFP_KERNEL);
> if (!pfn_sb)
> return -ENOMEM;
>
> @@ -711,11 +720,14 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
> sig = DAX_SIG;
> else
> sig = PFN_SIG;
> +
> rc = nd_pfn_validate(nd_pfn, sig);
> if (rc != -ENODEV)
> return rc;
>
> /* no info block, do init */;
> + memset(pfn_sb, 0, sizeof(*pfn_sb));
> +
> nd_region = to_nd_region(nd_pfn->dev.parent);
> if (nd_region->ro) {
> dev_info(&nd_pfn->dev,
> @@ -768,7 +780,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
> memcpy(pfn_sb->uuid, nd_pfn->uuid, 16);
> memcpy(pfn_sb->parent_uuid, nd_dev_to_uuid(&ndns->dev), 16);
> pfn_sb->version_major = cpu_to_le16(1);
> - pfn_sb->version_minor = cpu_to_le16(2);
> + pfn_sb->version_minor = cpu_to_le16(3);
> pfn_sb->start_pad = cpu_to_le32(start_pad);
> pfn_sb->end_trunc = cpu_to_le32(end_trunc);
> pfn_sb->align = cpu_to_le32(nd_pfn->align);
>
How will this minor version 3 be used? If we are not having
start_pad/end_trunc updated in pfn_sb, how will the older kernel enable these namesapces?
Do we need a patch like
https://lore.kernel.org/linux-mm/[email protected]
-aneesh
On 05.06.19 23:58, Dan Williams wrote:
> Prepare the memory hot-{add,remove} paths for handling sub-section
> ranges by plumbing the starting page frame and number of pages being
> handled through arch_{add,remove}_memory() to
> sparse_{add,remove}_one_section().
>
> This is simply plumbing, small cleanups, and some identifier renames. No
> intended functional changes.
>
> Cc: Michal Hocko <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Reviewed-by: Pavel Tatashin <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> include/linux/memory_hotplug.h | 5 +-
> mm/memory_hotplug.c | 114 +++++++++++++++++++++++++---------------
> mm/sparse.c | 15 ++---
> 3 files changed, 81 insertions(+), 53 deletions(-)
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 79e0add6a597..3ab0282b4fe5 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -348,9 +348,10 @@ extern int add_memory_resource(int nid, struct resource *resource);
> extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
> unsigned long nr_pages, struct vmem_altmap *altmap);
> extern bool is_memblock_offlined(struct memory_block *mem);
> -extern int sparse_add_one_section(int nid, unsigned long start_pfn,
> - struct vmem_altmap *altmap);
> +extern int sparse_add_section(int nid, unsigned long pfn,
> + unsigned long nr_pages, struct vmem_altmap *altmap);
> extern void sparse_remove_one_section(struct mem_section *ms,
> + unsigned long pfn, unsigned long nr_pages,
> unsigned long map_offset, struct vmem_altmap *altmap);
> extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
> unsigned long pnum);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 4b882c57781a..399bf78bccc5 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -252,51 +252,84 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
> }
> #endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
>
> -static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
> - struct vmem_altmap *altmap)
> +static int __meminit __add_section(int nid, unsigned long pfn,
> + unsigned long nr_pages, struct vmem_altmap *altmap)
> {
> int ret;
>
> - if (pfn_valid(phys_start_pfn))
> + if (pfn_valid(pfn))
> return -EEXIST;
>
> - ret = sparse_add_one_section(nid, phys_start_pfn, altmap);
> + ret = sparse_add_section(nid, pfn, nr_pages, altmap);
> return ret < 0 ? ret : 0;
> }
>
> +static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
> + const char *reason)
> +{
> + /*
> + * Disallow all operations smaller than a sub-section and only
> + * allow operations smaller than a section for
> + * SPARSEMEM_VMEMMAP. Note that check_hotplug_memory_range()
> + * enforces a larger memory_block_size_bytes() granularity for
> + * memory that will be marked online, so this check should only
> + * fire for direct arch_{add,remove}_memory() users outside of
> + * add_memory_resource().
> + */
> + unsigned long min_align;
> +
> + if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
> + min_align = PAGES_PER_SUBSECTION;
> + else
> + min_align = PAGES_PER_SECTION;
> + if (!IS_ALIGNED(pfn, min_align)
> + || !IS_ALIGNED(nr_pages, min_align)) {
> + WARN(1, "Misaligned __%s_pages start: %#lx end: #%lx\n",
> + reason, pfn, pfn + nr_pages - 1);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> /*
> * Reasonably generic function for adding memory. It is
> * expected that archs that support memory hotplug will
> * call this function after deciding the zone to which to
> * add the new pages.
> */
> -int __ref __add_pages(int nid, unsigned long phys_start_pfn,
> - unsigned long nr_pages, struct mhp_restrictions *restrictions)
> +int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> + struct mhp_restrictions *restrictions)
> {
> unsigned long i;
> - int err = 0;
> - int start_sec, end_sec;
> + int start_sec, end_sec, err;
> struct vmem_altmap *altmap = restrictions->altmap;
>
> - /* during initialize mem_map, align hot-added range to section */
> - start_sec = pfn_to_section_nr(phys_start_pfn);
> - end_sec = pfn_to_section_nr(phys_start_pfn + nr_pages - 1);
> -
> if (altmap) {
> /*
> * Validate altmap is within bounds of the total request
> */
> - if (altmap->base_pfn != phys_start_pfn
> + if (altmap->base_pfn != pfn
> || vmem_altmap_offset(altmap) > nr_pages) {
> pr_warn_once("memory add fail, invalid altmap\n");
> - err = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
> altmap->alloc = 0;
> }
>
> + err = check_pfn_span(pfn, nr_pages, "add");
> + if (err)
> + return err;
> +
> + start_sec = pfn_to_section_nr(pfn);
> + end_sec = pfn_to_section_nr(pfn + nr_pages - 1);
> for (i = start_sec; i <= end_sec; i++) {
> - err = __add_section(nid, section_nr_to_pfn(i), altmap);
> + unsigned long pfns;
> +
> + pfns = min(nr_pages, PAGES_PER_SECTION
> + - (pfn & ~PAGE_SECTION_MASK));
> + err = __add_section(nid, pfn, pfns, altmap);
> + pfn += pfns;
> + nr_pages -= pfns;
>
> /*
> * EEXIST is finally dealt with by ioresource collision
> @@ -309,7 +342,6 @@ int __ref __add_pages(int nid, unsigned long phys_start_pfn,
> cond_resched();
> }
> vmemmap_populate_print_last();
> -out:
> return err;
> }
>
> @@ -487,10 +519,10 @@ static void shrink_pgdat_span(struct pglist_data *pgdat,
> pgdat->node_spanned_pages = 0;
> }
>
> -static void __remove_zone(struct zone *zone, unsigned long start_pfn)
> +static void __remove_zone(struct zone *zone, unsigned long start_pfn,
> + unsigned long nr_pages)
> {
> struct pglist_data *pgdat = zone->zone_pgdat;
> - int nr_pages = PAGES_PER_SECTION;
> unsigned long flags;
>
> pgdat_resize_lock(zone->zone_pgdat, &flags);
> @@ -499,27 +531,23 @@ static void __remove_zone(struct zone *zone, unsigned long start_pfn)
> pgdat_resize_unlock(zone->zone_pgdat, &flags);
> }
>
> -static void __remove_section(struct zone *zone, struct mem_section *ms,
> - unsigned long map_offset,
> - struct vmem_altmap *altmap)
> +static void __remove_section(struct zone *zone, unsigned long pfn,
> + unsigned long nr_pages, unsigned long map_offset,
> + struct vmem_altmap *altmap)
> {
> - unsigned long start_pfn;
> - int scn_nr;
> + struct mem_section *ms = __nr_to_section(pfn_to_section_nr(pfn));
>
> if (WARN_ON_ONCE(!valid_section(ms)))
> return;
>
> - scn_nr = __section_nr(ms);
> - start_pfn = section_nr_to_pfn((unsigned long)scn_nr);
> - __remove_zone(zone, start_pfn);
> -
> - sparse_remove_one_section(ms, map_offset, altmap);
> + __remove_zone(zone, pfn, nr_pages);
> + sparse_remove_one_section(ms, pfn, nr_pages, map_offset, altmap);
> }
>
> /**
> * __remove_pages() - remove sections of pages from a zone
> * @zone: zone from which pages need to be removed
> - * @phys_start_pfn: starting pageframe (must be aligned to start of a section)
> + * @pfn: starting pageframe (must be aligned to start of a section)
> * @nr_pages: number of pages to remove (must be multiple of section size)
> * @altmap: alternative device page map or %NULL if default memmap is used
> *
> @@ -528,31 +556,31 @@ static void __remove_section(struct zone *zone, struct mem_section *ms,
> * sure that pages are marked reserved and zones are adjust properly by
> * calling offline_pages().
> */
> -void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
> +void __remove_pages(struct zone *zone, unsigned long pfn,
> unsigned long nr_pages, struct vmem_altmap *altmap)
> {
> - unsigned long i;
> unsigned long map_offset = 0;
> - int sections_to_remove;
> + int i, start_sec, end_sec;
Can you convert these to unsigned longs? I'll be sending a clenup that
will make this consistent tree-wide soon. Just making sure we won't miss
this.
--
Thanks,
David / dhildenb
Dan Williams <[email protected]> writes:
> Allow sub-section sized ranges to be added to the memmap.
> populate_section_memmap() takes an explict pfn range rather than
> assuming a full section, and those parameters are plumbed all the way
> through to vmmemap_populate(). There should be no sub-section usage in
> current deployments. New warnings are added to clarify which memmap
> allocation paths are sub-section capable.
>
> Cc: Michal Hocko <[email protected]>
> Cc: David Hildenbrand <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Reviewed-by: Pavel Tatashin <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> arch/x86/mm/init_64.c | 4 ++-
> include/linux/mm.h | 4 ++-
> mm/sparse-vmemmap.c | 21 +++++++++++------
> mm/sparse.c | 61 +++++++++++++++++++++++++++++++------------------
> 4 files changed, 57 insertions(+), 33 deletions(-)
>
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 8335ac6e1112..688fb0687e55 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1520,7 +1520,9 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
> {
> int err;
>
> - if (boot_cpu_has(X86_FEATURE_PSE))
> + if (end - start < PAGES_PER_SECTION * sizeof(struct page))
> + err = vmemmap_populate_basepages(start, end, node);
> + else if (boot_cpu_has(X86_FEATURE_PSE))
> err = vmemmap_populate_hugepages(start, end, node, altmap);
> else if (altmap) {
> pr_err_once("%s: no cpu support for altmap allocations\n",
Can we move this to another patch? I am wondering what the x86 behaviour
here is? If the range is less that PAGES_PER_SECTION we don't allow to
use pmem as the map device? We sliently use memory range?
-aneesh
Dan Williams <[email protected]> writes:
> Teach devm_memremap_pages() about the new sub-section capabilities of
> arch_{add,remove}_memory(). Effectively, just replace all usage of
> align_start, align_end, and align_size with res->start, res->end, and
> resource_size(res). The existing sanity check will still make sure that
> the two separate remap attempts do not collide within a sub-section (2MB
> on x86).
>
> Cc: Michal Hocko <[email protected]>
> Cc: Toshi Kani <[email protected]>
> Cc: Jérôme Glisse <[email protected]>
> Cc: Logan Gunthorpe <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Cc: Pavel Tatashin <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> ---
> kernel/memremap.c | 61 +++++++++++++++++++++--------------------------------
> 1 file changed, 24 insertions(+), 37 deletions(-)
>
> diff --git a/kernel/memremap.c b/kernel/memremap.c
> index 57980ed4e571..a0e5f6b91b04 100644
> --- a/kernel/memremap.c
> +++ b/kernel/memremap.c
> @@ -58,7 +58,7 @@ static unsigned long pfn_first(struct dev_pagemap *pgmap)
> struct vmem_altmap *altmap = &pgmap->altmap;
> unsigned long pfn;
>
> - pfn = res->start >> PAGE_SHIFT;
> + pfn = PHYS_PFN(res->start);
> if (pgmap->altmap_valid)
> pfn += vmem_altmap_offset(altmap);
> return pfn;
> @@ -86,7 +86,6 @@ static void devm_memremap_pages_release(void *data)
> struct dev_pagemap *pgmap = data;
> struct device *dev = pgmap->dev;
> struct resource *res = &pgmap->res;
> - resource_size_t align_start, align_size;
> unsigned long pfn;
> int nid;
>
> @@ -96,25 +95,21 @@ static void devm_memremap_pages_release(void *data)
> pgmap->cleanup(pgmap->ref);
>
> /* pages are dead and unused, undo the arch mapping */
> - align_start = res->start & ~(PA_SECTION_SIZE - 1);
> - align_size = ALIGN(res->start + resource_size(res), PA_SECTION_SIZE)
> - - align_start;
> -
> - nid = page_to_nid(pfn_to_page(align_start >> PAGE_SHIFT));
> + nid = page_to_nid(pfn_to_page(PHYS_PFN(res->start)));
Why do we not require to align things to subsection size now?
-aneesh
On Wed, Jun 05, 2019 at 02:57:54PM -0700, Dan Williams wrote:
>Towards enabling memory hotplug to track partial population of a
>section, introduce 'struct mem_section_usage'.
>
>A pointer to a 'struct mem_section_usage' instance replaces the existing
>pointer to a 'pageblock_flags' bitmap. Effectively it adds one more
>'unsigned long' beyond the 'pageblock_flags' (usemap) allocation to
>house a new 'subsection_map' bitmap. The new bitmap enables the memory
>hot{plug,remove} implementation to act on incremental sub-divisions of a
>section.
>
>The default SUBSECTION_SHIFT is chosen to keep the 'subsection_map' no
>larger than a single 'unsigned long' on the major architectures.
>Alternatively an architecture can define ARCH_SUBSECTION_SHIFT to
>override the default PMD_SHIFT. Note that PowerPC needs to use
>ARCH_SUBSECTION_SHIFT to workaround PMD_SHIFT being a non-constant
>expression on PowerPC.
>
>The primary motivation for this functionality is to support platforms
>that mix "System RAM" and "Persistent Memory" within a single section,
>or multiple PMEM ranges with different mapping lifetimes within a single
>section. The section restriction for hotplug has caused an ongoing saga
>of hacks and bugs for devm_memremap_pages() users.
>
>Beyond the fixups to teach existing paths how to retrieve the 'usemap'
>from a section, and updates to usemap allocation path, there are no
>expected behavior changes.
>
>Cc: Michal Hocko <[email protected]>
>Cc: Vlastimil Babka <[email protected]>
>Cc: Logan Gunthorpe <[email protected]>
>Cc: Oscar Salvador <[email protected]>
>Cc: Pavel Tatashin <[email protected]>
>Cc: Benjamin Herrenschmidt <[email protected]>
>Cc: Paul Mackerras <[email protected]>
>Cc: Michael Ellerman <[email protected]>
>Signed-off-by: Dan Williams <[email protected]>
>---
> arch/powerpc/include/asm/sparsemem.h | 3 +
> include/linux/mmzone.h | 48 +++++++++++++++++++-
> mm/memory_hotplug.c | 18 ++++----
> mm/page_alloc.c | 2 -
> mm/sparse.c | 81 +++++++++++++++++-----------------
> 5 files changed, 99 insertions(+), 53 deletions(-)
>
>diff --git a/arch/powerpc/include/asm/sparsemem.h b/arch/powerpc/include/asm/sparsemem.h
>index 3192d454a733..1aa3c9303bf8 100644
>--- a/arch/powerpc/include/asm/sparsemem.h
>+++ b/arch/powerpc/include/asm/sparsemem.h
>@@ -10,6 +10,9 @@
> */
> #define SECTION_SIZE_BITS 24
>
>+/* Reflect the largest possible PMD-size as the subsection-size constant */
>+#define ARCH_SUBSECTION_SHIFT 24
>+
> #endif /* CONFIG_SPARSEMEM */
>
> #ifdef CONFIG_MEMORY_HOTPLUG
>diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>index 427b79c39b3c..ac163f2f274f 100644
>--- a/include/linux/mmzone.h
>+++ b/include/linux/mmzone.h
>@@ -1161,6 +1161,44 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
> #define SECTION_ALIGN_UP(pfn) (((pfn) + PAGES_PER_SECTION - 1) & PAGE_SECTION_MASK)
> #define SECTION_ALIGN_DOWN(pfn) ((pfn) & PAGE_SECTION_MASK)
>
>+/*
>+ * SUBSECTION_SHIFT must be constant since it is used to declare
>+ * subsection_map and related bitmaps without triggering the generation
>+ * of variable-length arrays. The most natural size for a subsection is
>+ * a PMD-page. For architectures that do not have a constant PMD-size
>+ * ARCH_SUBSECTION_SHIFT can be set to a constant max size, or otherwise
>+ * fallback to 2MB.
>+ */
>+#if defined(ARCH_SUBSECTION_SHIFT)
>+#define SUBSECTION_SHIFT (ARCH_SUBSECTION_SHIFT)
>+#elif defined(PMD_SHIFT)
>+#define SUBSECTION_SHIFT (PMD_SHIFT)
>+#else
>+/*
>+ * Memory hotplug enabled platforms avoid this default because they
>+ * either define ARCH_SUBSECTION_SHIFT, or PMD_SHIFT is a constant, but
>+ * this is kept as a backstop to allow compilation on
>+ * !ARCH_ENABLE_MEMORY_HOTPLUG archs.
>+ */
>+#define SUBSECTION_SHIFT 21
>+#endif
>+
>+#define PFN_SUBSECTION_SHIFT (SUBSECTION_SHIFT - PAGE_SHIFT)
>+#define PAGES_PER_SUBSECTION (1UL << PFN_SUBSECTION_SHIFT)
>+#define PAGE_SUBSECTION_MASK ((~(PAGES_PER_SUBSECTION-1)))
One pair of brackets could be removed, IMHO.
>+
>+#if SUBSECTION_SHIFT > SECTION_SIZE_BITS
>+#error Subsection size exceeds section size
>+#else
>+#define SUBSECTIONS_PER_SECTION (1UL << (SECTION_SIZE_BITS - SUBSECTION_SHIFT))
>+#endif
>+
>+struct mem_section_usage {
>+ DECLARE_BITMAP(subsection_map, SUBSECTIONS_PER_SECTION);
>+ /* See declaration of similar field in struct zone */
>+ unsigned long pageblock_flags[0];
>+};
>+
> struct page;
> struct page_ext;
> struct mem_section {
>@@ -1178,8 +1216,7 @@ struct mem_section {
> */
> unsigned long section_mem_map;
>
>- /* See declaration of similar field in struct zone */
>- unsigned long *pageblock_flags;
>+ struct mem_section_usage *usage;
> #ifdef CONFIG_PAGE_EXTENSION
> /*
> * If SPARSEMEM, pgdat doesn't have page_ext pointer. We use
>@@ -1210,6 +1247,11 @@ extern struct mem_section **mem_section;
> extern struct mem_section mem_section[NR_SECTION_ROOTS][SECTIONS_PER_ROOT];
> #endif
>
>+static inline unsigned long *section_to_usemap(struct mem_section *ms)
>+{
>+ return ms->usage->pageblock_flags;
Do we need to consider the case when ms->usage is NULL?
>+}
>+
> static inline struct mem_section *__nr_to_section(unsigned long nr)
> {
> #ifdef CONFIG_SPARSEMEM_EXTREME
>@@ -1221,7 +1263,7 @@ static inline struct mem_section *__nr_to_section(unsigned long nr)
> return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK];
> }
> extern int __section_nr(struct mem_section* ms);
>-extern unsigned long usemap_size(void);
>+extern size_t mem_section_usage_size(void);
>
> /*
> * We use the lower bits of the mem_map pointer to store
>diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>index a88c5f334e5a..7b963c2d3a0d 100644
>--- a/mm/memory_hotplug.c
>+++ b/mm/memory_hotplug.c
>@@ -166,9 +166,10 @@ void put_page_bootmem(struct page *page)
> #ifndef CONFIG_SPARSEMEM_VMEMMAP
> static void register_page_bootmem_info_section(unsigned long start_pfn)
> {
>- unsigned long *usemap, mapsize, section_nr, i;
>+ unsigned long mapsize, section_nr, i;
> struct mem_section *ms;
> struct page *page, *memmap;
>+ struct mem_section_usage *usage;
>
> section_nr = pfn_to_section_nr(start_pfn);
> ms = __nr_to_section(section_nr);
>@@ -188,10 +189,10 @@ static void register_page_bootmem_info_section(unsigned long start_pfn)
> for (i = 0; i < mapsize; i++, page++)
> get_page_bootmem(section_nr, page, SECTION_INFO);
>
>- usemap = ms->pageblock_flags;
>- page = virt_to_page(usemap);
>+ usage = ms->usage;
>+ page = virt_to_page(usage);
>
>- mapsize = PAGE_ALIGN(usemap_size()) >> PAGE_SHIFT;
>+ mapsize = PAGE_ALIGN(mem_section_usage_size()) >> PAGE_SHIFT;
>
> for (i = 0; i < mapsize; i++, page++)
> get_page_bootmem(section_nr, page, MIX_SECTION_INFO);
>@@ -200,9 +201,10 @@ static void register_page_bootmem_info_section(unsigned long start_pfn)
> #else /* CONFIG_SPARSEMEM_VMEMMAP */
> static void register_page_bootmem_info_section(unsigned long start_pfn)
> {
>- unsigned long *usemap, mapsize, section_nr, i;
>+ unsigned long mapsize, section_nr, i;
> struct mem_section *ms;
> struct page *page, *memmap;
>+ struct mem_section_usage *usage;
>
> section_nr = pfn_to_section_nr(start_pfn);
> ms = __nr_to_section(section_nr);
>@@ -211,10 +213,10 @@ static void register_page_bootmem_info_section(unsigned long start_pfn)
>
> register_page_bootmem_memmap(section_nr, memmap, PAGES_PER_SECTION);
>
>- usemap = ms->pageblock_flags;
>- page = virt_to_page(usemap);
>+ usage = ms->usage;
>+ page = virt_to_page(usage);
>
>- mapsize = PAGE_ALIGN(usemap_size()) >> PAGE_SHIFT;
>+ mapsize = PAGE_ALIGN(mem_section_usage_size()) >> PAGE_SHIFT;
>
> for (i = 0; i < mapsize; i++, page++)
> get_page_bootmem(section_nr, page, MIX_SECTION_INFO);
>diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>index c061f66c2d0c..c6d8224d792e 100644
>--- a/mm/page_alloc.c
>+++ b/mm/page_alloc.c
>@@ -404,7 +404,7 @@ static inline unsigned long *get_pageblock_bitmap(struct page *page,
> unsigned long pfn)
> {
> #ifdef CONFIG_SPARSEMEM
>- return __pfn_to_section(pfn)->pageblock_flags;
>+ return section_to_usemap(__pfn_to_section(pfn));
> #else
> return page_zone(page)->pageblock_flags;
> #endif /* CONFIG_SPARSEMEM */
>diff --git a/mm/sparse.c b/mm/sparse.c
>index 1552c855d62a..71da15cc7432 100644
>--- a/mm/sparse.c
>+++ b/mm/sparse.c
>@@ -288,33 +288,31 @@ struct page *sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long pn
>
> static void __meminit sparse_init_one_section(struct mem_section *ms,
> unsigned long pnum, struct page *mem_map,
>- unsigned long *pageblock_bitmap)
>+ struct mem_section_usage *usage)
> {
> ms->section_mem_map &= ~SECTION_MAP_MASK;
> ms->section_mem_map |= sparse_encode_mem_map(mem_map, pnum) |
> SECTION_HAS_MEM_MAP;
>- ms->pageblock_flags = pageblock_bitmap;
>+ ms->usage = usage;
> }
>
>-unsigned long usemap_size(void)
>+static unsigned long usemap_size(void)
> {
> return BITS_TO_LONGS(SECTION_BLOCKFLAGS_BITS) * sizeof(unsigned long);
> }
>
>-#ifdef CONFIG_MEMORY_HOTPLUG
>-static unsigned long *__kmalloc_section_usemap(void)
>+size_t mem_section_usage_size(void)
> {
>- return kmalloc(usemap_size(), GFP_KERNEL);
>+ return sizeof(struct mem_section_usage) + usemap_size();
> }
>-#endif /* CONFIG_MEMORY_HOTPLUG */
>
> #ifdef CONFIG_MEMORY_HOTREMOVE
>-static unsigned long * __init
>+static struct mem_section_usage * __init
> sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat,
> unsigned long size)
> {
>+ struct mem_section_usage *usage;
> unsigned long goal, limit;
>- unsigned long *p;
> int nid;
> /*
> * A page may contain usemaps for other sections preventing the
>@@ -330,15 +328,16 @@ sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat,
> limit = goal + (1UL << PA_SECTION_SHIFT);
> nid = early_pfn_to_nid(goal >> PAGE_SHIFT);
> again:
>- p = memblock_alloc_try_nid(size, SMP_CACHE_BYTES, goal, limit, nid);
>- if (!p && limit) {
>+ usage = memblock_alloc_try_nid(size, SMP_CACHE_BYTES, goal, limit, nid);
>+ if (!usage && limit) {
> limit = 0;
> goto again;
> }
>- return p;
>+ return usage;
> }
>
>-static void __init check_usemap_section_nr(int nid, unsigned long *usemap)
>+static void __init check_usemap_section_nr(int nid,
>+ struct mem_section_usage *usage)
> {
> unsigned long usemap_snr, pgdat_snr;
> static unsigned long old_usemap_snr;
>@@ -352,7 +351,7 @@ static void __init check_usemap_section_nr(int nid, unsigned long *usemap)
> old_pgdat_snr = NR_MEM_SECTIONS;
> }
>
>- usemap_snr = pfn_to_section_nr(__pa(usemap) >> PAGE_SHIFT);
>+ usemap_snr = pfn_to_section_nr(__pa(usage) >> PAGE_SHIFT);
> pgdat_snr = pfn_to_section_nr(__pa(pgdat) >> PAGE_SHIFT);
> if (usemap_snr == pgdat_snr)
> return;
>@@ -380,14 +379,15 @@ static void __init check_usemap_section_nr(int nid, unsigned long *usemap)
> usemap_snr, pgdat_snr, nid);
> }
> #else
>-static unsigned long * __init
>+static struct mem_section_usage * __init
> sparse_early_usemaps_alloc_pgdat_section(struct pglist_data *pgdat,
> unsigned long size)
> {
> return memblock_alloc_node(size, SMP_CACHE_BYTES, pgdat->node_id);
> }
>
>-static void __init check_usemap_section_nr(int nid, unsigned long *usemap)
>+static void __init check_usemap_section_nr(int nid,
>+ struct mem_section_usage *usage)
> {
> }
> #endif /* CONFIG_MEMORY_HOTREMOVE */
>@@ -474,14 +474,13 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,
> unsigned long pnum_end,
> unsigned long map_count)
> {
>- unsigned long pnum, usemap_longs, *usemap;
>+ struct mem_section_usage *usage;
>+ unsigned long pnum;
> struct page *map;
>
>- usemap_longs = BITS_TO_LONGS(SECTION_BLOCKFLAGS_BITS);
>- usemap = sparse_early_usemaps_alloc_pgdat_section(NODE_DATA(nid),
>- usemap_size() *
>- map_count);
>- if (!usemap) {
>+ usage = sparse_early_usemaps_alloc_pgdat_section(NODE_DATA(nid),
>+ mem_section_usage_size() * map_count);
>+ if (!usage) {
> pr_err("%s: node[%d] usemap allocation failed", __func__, nid);
> goto failed;
> }
>@@ -497,9 +496,9 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,
> pnum_begin = pnum;
> goto failed;
> }
>- check_usemap_section_nr(nid, usemap);
>- sparse_init_one_section(__nr_to_section(pnum), pnum, map, usemap);
>- usemap += usemap_longs;
>+ check_usemap_section_nr(nid, usage);
>+ sparse_init_one_section(__nr_to_section(pnum), pnum, map, usage);
>+ usage = (void *) usage + mem_section_usage_size();
> }
> sparse_buffer_fini();
> return;
>@@ -697,9 +696,9 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
> struct vmem_altmap *altmap)
> {
> unsigned long section_nr = pfn_to_section_nr(start_pfn);
>+ struct mem_section_usage *usage;
> struct mem_section *ms;
> struct page *memmap;
>- unsigned long *usemap;
> int ret;
>
> /*
>@@ -713,8 +712,8 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
> memmap = kmalloc_section_memmap(section_nr, nid, altmap);
> if (!memmap)
> return -ENOMEM;
>- usemap = __kmalloc_section_usemap();
>- if (!usemap) {
>+ usage = kzalloc(mem_section_usage_size(), GFP_KERNEL);
>+ if (!usage) {
> __kfree_section_memmap(memmap, altmap);
> return -ENOMEM;
> }
>@@ -732,11 +731,11 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn,
> page_init_poison(memmap, sizeof(struct page) * PAGES_PER_SECTION);
>
> section_mark_present(ms);
>- sparse_init_one_section(ms, section_nr, memmap, usemap);
>+ sparse_init_one_section(ms, section_nr, memmap, usage);
>
> out:
> if (ret < 0) {
>- kfree(usemap);
>+ kfree(usage);
> __kfree_section_memmap(memmap, altmap);
> }
> return ret;
>@@ -772,20 +771,20 @@ static inline void clear_hwpoisoned_pages(struct page *memmap, int nr_pages)
> }
> #endif
>
>-static void free_section_usemap(struct page *memmap, unsigned long *usemap,
>- struct vmem_altmap *altmap)
>+static void free_section_usage(struct page *memmap,
>+ struct mem_section_usage *usage, struct vmem_altmap *altmap)
> {
>- struct page *usemap_page;
>+ struct page *usage_page;
>
>- if (!usemap)
>+ if (!usage)
> return;
>
>- usemap_page = virt_to_page(usemap);
>+ usage_page = virt_to_page(usage);
> /*
> * Check to see if allocation came from hot-plug-add
> */
>- if (PageSlab(usemap_page) || PageCompound(usemap_page)) {
>- kfree(usemap);
>+ if (PageSlab(usage_page) || PageCompound(usage_page)) {
>+ kfree(usage);
> if (memmap)
> __kfree_section_memmap(memmap, altmap);
> return;
>@@ -804,18 +803,18 @@ void sparse_remove_one_section(struct mem_section *ms, unsigned long map_offset,
> struct vmem_altmap *altmap)
> {
> struct page *memmap = NULL;
>- unsigned long *usemap = NULL;
>+ struct mem_section_usage *usage = NULL;
>
> if (ms->section_mem_map) {
>- usemap = ms->pageblock_flags;
>+ usage = ms->usage;
> memmap = sparse_decode_mem_map(ms->section_mem_map,
> __section_nr(ms));
> ms->section_mem_map = 0;
>- ms->pageblock_flags = NULL;
>+ ms->usage = NULL;
> }
>
> clear_hwpoisoned_pages(memmap + map_offset,
> PAGES_PER_SECTION - map_offset);
>- free_section_usemap(memmap, usemap, altmap);
>+ free_section_usage(memmap, usage, altmap);
> }
> #endif /* CONFIG_MEMORY_HOTPLUG */
--
Wei Yang
Help you, Help me
On Wed, Jun 05, 2019 at 02:57:59PM -0700, Dan Williams wrote:
>Prepare for hot{plug,remove} of sub-ranges of a section by tracking a
>sub-section active bitmask, each bit representing a PMD_SIZE span of the
>architecture's memory hotplug section size.
>
>The implications of a partially populated section is that pfn_valid()
>needs to go beyond a valid_section() check and read the sub-section
>active ranges from the bitmask. The expectation is that the bitmask
>(subsection_map) fits in the same cacheline as the valid_section() data,
>so the incremental performance overhead to pfn_valid() should be
>negligible.
>
>Cc: Michal Hocko <[email protected]>
>Cc: Vlastimil Babka <[email protected]>
>Cc: Logan Gunthorpe <[email protected]>
>Cc: Oscar Salvador <[email protected]>
>Cc: Pavel Tatashin <[email protected]>
>Tested-by: Jane Chu <[email protected]>
>Signed-off-by: Dan Williams <[email protected]>
>---
> include/linux/mmzone.h | 29 ++++++++++++++++++++++++++++-
> mm/page_alloc.c | 4 +++-
> mm/sparse.c | 35 +++++++++++++++++++++++++++++++++++
> 3 files changed, 66 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>index ac163f2f274f..6dd52d544857 100644
>--- a/include/linux/mmzone.h
>+++ b/include/linux/mmzone.h
>@@ -1199,6 +1199,8 @@ struct mem_section_usage {
> unsigned long pageblock_flags[0];
> };
>
>+void subsection_map_init(unsigned long pfn, unsigned long nr_pages);
>+
> struct page;
> struct page_ext;
> struct mem_section {
>@@ -1336,12 +1338,36 @@ static inline struct mem_section *__pfn_to_section(unsigned long pfn)
>
> extern int __highest_present_section_nr;
>
>+static inline int subsection_map_index(unsigned long pfn)
>+{
>+ return (pfn & ~(PAGE_SECTION_MASK)) / PAGES_PER_SUBSECTION;
>+}
>+
>+#ifdef CONFIG_SPARSEMEM_VMEMMAP
>+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
>+{
>+ int idx = subsection_map_index(pfn);
>+
>+ return test_bit(idx, ms->usage->subsection_map);
>+}
>+#else
>+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
>+{
>+ return 1;
>+}
>+#endif
>+
> #ifndef CONFIG_HAVE_ARCH_PFN_VALID
> static inline int pfn_valid(unsigned long pfn)
> {
>+ struct mem_section *ms;
>+
> if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
> return 0;
>- return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
>+ ms = __nr_to_section(pfn_to_section_nr(pfn));
>+ if (!valid_section(ms))
>+ return 0;
>+ return pfn_section_valid(ms, pfn);
> }
> #endif
>
>@@ -1373,6 +1399,7 @@ void sparse_init(void);
> #define sparse_init() do {} while (0)
> #define sparse_index_init(_sec, _nid) do {} while (0)
> #define pfn_present pfn_valid
>+#define subsection_map_init(_pfn, _nr_pages) do {} while (0)
> #endif /* CONFIG_SPARSEMEM */
>
> /*
>diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>index c6d8224d792e..bd773efe5b82 100644
>--- a/mm/page_alloc.c
>+++ b/mm/page_alloc.c
>@@ -7292,10 +7292,12 @@ void __init free_area_init_nodes(unsigned long *max_zone_pfn)
>
> /* Print out the early node map */
> pr_info("Early memory node ranges\n");
>- for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid)
>+ for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid) {
> pr_info(" node %3d: [mem %#018Lx-%#018Lx]\n", nid,
> (u64)start_pfn << PAGE_SHIFT,
> ((u64)end_pfn << PAGE_SHIFT) - 1);
>+ subsection_map_init(start_pfn, end_pfn - start_pfn);
>+ }
Just curious about why we set subsection here?
Function free_area_init_nodes() mostly handles pgdat, if I am correct. Setup
subsection here looks like touching some lower level system data structure.
>
> /* Initialise every node */
> mminit_verify_pageflags_layout();
>diff --git a/mm/sparse.c b/mm/sparse.c
>index 71da15cc7432..0baa2e55cfdd 100644
>--- a/mm/sparse.c
>+++ b/mm/sparse.c
>@@ -210,6 +210,41 @@ static inline unsigned long first_present_section_nr(void)
> return next_present_section_nr(-1);
> }
>
>+void subsection_mask_set(unsigned long *map, unsigned long pfn,
>+ unsigned long nr_pages)
>+{
>+ int idx = subsection_map_index(pfn);
>+ int end = subsection_map_index(pfn + nr_pages - 1);
>+
>+ bitmap_set(map, idx, end - idx + 1);
>+}
>+
>+void subsection_map_init(unsigned long pfn, unsigned long nr_pages)
>+{
>+ int end_sec = pfn_to_section_nr(pfn + nr_pages - 1);
>+ int i, start_sec = pfn_to_section_nr(pfn);
>+
>+ if (!nr_pages)
>+ return;
>+
>+ for (i = start_sec; i <= end_sec; i++) {
>+ struct mem_section *ms;
>+ unsigned long pfns;
>+
>+ pfns = min(nr_pages, PAGES_PER_SECTION
>+ - (pfn & ~PAGE_SECTION_MASK));
>+ ms = __nr_to_section(i);
>+ subsection_mask_set(ms->usage->subsection_map, pfn, pfns);
>+
>+ pr_debug("%s: sec: %d pfns: %ld set(%d, %d)\n", __func__, i,
>+ pfns, subsection_map_index(pfn),
>+ subsection_map_index(pfn + pfns - 1));
>+
>+ pfn += pfns;
>+ nr_pages -= pfns;
>+ }
>+}
>+
> /* Record a memory area against a node. */
> void __init memory_present(int nid, unsigned long start, unsigned long end)
> {
--
Wei Yang
Help you, Help me
On Mon, Jun 17, 2019 at 3:22 PM Wei Yang <[email protected]> wrote:
>
> On Wed, Jun 05, 2019 at 02:57:59PM -0700, Dan Williams wrote:
> >Prepare for hot{plug,remove} of sub-ranges of a section by tracking a
> >sub-section active bitmask, each bit representing a PMD_SIZE span of the
> >architecture's memory hotplug section size.
> >
> >The implications of a partially populated section is that pfn_valid()
> >needs to go beyond a valid_section() check and read the sub-section
> >active ranges from the bitmask. The expectation is that the bitmask
> >(subsection_map) fits in the same cacheline as the valid_section() data,
> >so the incremental performance overhead to pfn_valid() should be
> >negligible.
> >
> >Cc: Michal Hocko <[email protected]>
> >Cc: Vlastimil Babka <[email protected]>
> >Cc: Logan Gunthorpe <[email protected]>
> >Cc: Oscar Salvador <[email protected]>
> >Cc: Pavel Tatashin <[email protected]>
> >Tested-by: Jane Chu <[email protected]>
> >Signed-off-by: Dan Williams <[email protected]>
> >---
> > include/linux/mmzone.h | 29 ++++++++++++++++++++++++++++-
> > mm/page_alloc.c | 4 +++-
> > mm/sparse.c | 35 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 66 insertions(+), 2 deletions(-)
> >
> >diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> >index ac163f2f274f..6dd52d544857 100644
> >--- a/include/linux/mmzone.h
> >+++ b/include/linux/mmzone.h
> >@@ -1199,6 +1199,8 @@ struct mem_section_usage {
> > unsigned long pageblock_flags[0];
> > };
> >
> >+void subsection_map_init(unsigned long pfn, unsigned long nr_pages);
> >+
> > struct page;
> > struct page_ext;
> > struct mem_section {
> >@@ -1336,12 +1338,36 @@ static inline struct mem_section *__pfn_to_section(unsigned long pfn)
> >
> > extern int __highest_present_section_nr;
> >
> >+static inline int subsection_map_index(unsigned long pfn)
> >+{
> >+ return (pfn & ~(PAGE_SECTION_MASK)) / PAGES_PER_SUBSECTION;
> >+}
> >+
> >+#ifdef CONFIG_SPARSEMEM_VMEMMAP
> >+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> >+{
> >+ int idx = subsection_map_index(pfn);
> >+
> >+ return test_bit(idx, ms->usage->subsection_map);
> >+}
> >+#else
> >+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> >+{
> >+ return 1;
> >+}
> >+#endif
> >+
> > #ifndef CONFIG_HAVE_ARCH_PFN_VALID
> > static inline int pfn_valid(unsigned long pfn)
> > {
> >+ struct mem_section *ms;
> >+
> > if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
> > return 0;
> >- return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
> >+ ms = __nr_to_section(pfn_to_section_nr(pfn));
> >+ if (!valid_section(ms))
> >+ return 0;
> >+ return pfn_section_valid(ms, pfn);
> > }
> > #endif
> >
> >@@ -1373,6 +1399,7 @@ void sparse_init(void);
> > #define sparse_init() do {} while (0)
> > #define sparse_index_init(_sec, _nid) do {} while (0)
> > #define pfn_present pfn_valid
> >+#define subsection_map_init(_pfn, _nr_pages) do {} while (0)
> > #endif /* CONFIG_SPARSEMEM */
> >
> > /*
> >diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >index c6d8224d792e..bd773efe5b82 100644
> >--- a/mm/page_alloc.c
> >+++ b/mm/page_alloc.c
> >@@ -7292,10 +7292,12 @@ void __init free_area_init_nodes(unsigned long *max_zone_pfn)
> >
> > /* Print out the early node map */
> > pr_info("Early memory node ranges\n");
> >- for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid)
> >+ for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid) {
> > pr_info(" node %3d: [mem %#018Lx-%#018Lx]\n", nid,
> > (u64)start_pfn << PAGE_SHIFT,
> > ((u64)end_pfn << PAGE_SHIFT) - 1);
> >+ subsection_map_init(start_pfn, end_pfn - start_pfn);
> >+ }
>
> Just curious about why we set subsection here?
>
> Function free_area_init_nodes() mostly handles pgdat, if I am correct. Setup
> subsection here looks like touching some lower level system data structure.
Correct, I'm not sure how it ended up there, but it was the source of
a bug that was fixed with this change:
https://lore.kernel.org/lkml/CAPcyv4hjvBPDYKpp2Gns3-cc2AQ0AVS1nLk-K3fwXeRUvvzQLg@mail.gmail.com/
On Mon, Jun 17, 2019 at 03:32:45PM -0700, Dan Williams wrote:
>On Mon, Jun 17, 2019 at 3:22 PM Wei Yang <[email protected]> wrote:
>>
>> On Wed, Jun 05, 2019 at 02:57:59PM -0700, Dan Williams wrote:
>> >Prepare for hot{plug,remove} of sub-ranges of a section by tracking a
>> >sub-section active bitmask, each bit representing a PMD_SIZE span of the
>> >architecture's memory hotplug section size.
>> >
>> >The implications of a partially populated section is that pfn_valid()
>> >needs to go beyond a valid_section() check and read the sub-section
>> >active ranges from the bitmask. The expectation is that the bitmask
>> >(subsection_map) fits in the same cacheline as the valid_section() data,
>> >so the incremental performance overhead to pfn_valid() should be
>> >negligible.
>> >
>> >Cc: Michal Hocko <[email protected]>
>> >Cc: Vlastimil Babka <[email protected]>
>> >Cc: Logan Gunthorpe <[email protected]>
>> >Cc: Oscar Salvador <[email protected]>
>> >Cc: Pavel Tatashin <[email protected]>
>> >Tested-by: Jane Chu <[email protected]>
>> >Signed-off-by: Dan Williams <[email protected]>
>> >---
>> > include/linux/mmzone.h | 29 ++++++++++++++++++++++++++++-
>> > mm/page_alloc.c | 4 +++-
>> > mm/sparse.c | 35 +++++++++++++++++++++++++++++++++++
>> > 3 files changed, 66 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> >index ac163f2f274f..6dd52d544857 100644
>> >--- a/include/linux/mmzone.h
>> >+++ b/include/linux/mmzone.h
>> >@@ -1199,6 +1199,8 @@ struct mem_section_usage {
>> > unsigned long pageblock_flags[0];
>> > };
>> >
>> >+void subsection_map_init(unsigned long pfn, unsigned long nr_pages);
>> >+
>> > struct page;
>> > struct page_ext;
>> > struct mem_section {
>> >@@ -1336,12 +1338,36 @@ static inline struct mem_section *__pfn_to_section(unsigned long pfn)
>> >
>> > extern int __highest_present_section_nr;
>> >
>> >+static inline int subsection_map_index(unsigned long pfn)
>> >+{
>> >+ return (pfn & ~(PAGE_SECTION_MASK)) / PAGES_PER_SUBSECTION;
>> >+}
>> >+
>> >+#ifdef CONFIG_SPARSEMEM_VMEMMAP
>> >+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
>> >+{
>> >+ int idx = subsection_map_index(pfn);
>> >+
>> >+ return test_bit(idx, ms->usage->subsection_map);
>> >+}
>> >+#else
>> >+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
>> >+{
>> >+ return 1;
>> >+}
>> >+#endif
>> >+
>> > #ifndef CONFIG_HAVE_ARCH_PFN_VALID
>> > static inline int pfn_valid(unsigned long pfn)
>> > {
>> >+ struct mem_section *ms;
>> >+
>> > if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
>> > return 0;
>> >- return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
>> >+ ms = __nr_to_section(pfn_to_section_nr(pfn));
>> >+ if (!valid_section(ms))
>> >+ return 0;
>> >+ return pfn_section_valid(ms, pfn);
>> > }
>> > #endif
>> >
>> >@@ -1373,6 +1399,7 @@ void sparse_init(void);
>> > #define sparse_init() do {} while (0)
>> > #define sparse_index_init(_sec, _nid) do {} while (0)
>> > #define pfn_present pfn_valid
>> >+#define subsection_map_init(_pfn, _nr_pages) do {} while (0)
>> > #endif /* CONFIG_SPARSEMEM */
>> >
>> > /*
>> >diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> >index c6d8224d792e..bd773efe5b82 100644
>> >--- a/mm/page_alloc.c
>> >+++ b/mm/page_alloc.c
>> >@@ -7292,10 +7292,12 @@ void __init free_area_init_nodes(unsigned long *max_zone_pfn)
>> >
>> > /* Print out the early node map */
>> > pr_info("Early memory node ranges\n");
>> >- for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid)
>> >+ for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid) {
>> > pr_info(" node %3d: [mem %#018Lx-%#018Lx]\n", nid,
>> > (u64)start_pfn << PAGE_SHIFT,
>> > ((u64)end_pfn << PAGE_SHIFT) - 1);
>> >+ subsection_map_init(start_pfn, end_pfn - start_pfn);
>> >+ }
>>
>> Just curious about why we set subsection here?
>>
>> Function free_area_init_nodes() mostly handles pgdat, if I am correct. Setup
>> subsection here looks like touching some lower level system data structure.
>
>Correct, I'm not sure how it ended up there, but it was the source of
>a bug that was fixed with this change:
>
>https://lore.kernel.org/lkml/CAPcyv4hjvBPDYKpp2Gns3-cc2AQ0AVS1nLk-K3fwXeRUvvzQLg@mail.gmail.com/
So this one is moved to sparse_init_nid().
The bug is strange, while the code now is more reasonable to me.
Thanks :-)
>_______________________________________________
>Linux-nvdimm mailing list
>[email protected]
>https://lists.01.org/mailman/listinfo/linux-nvdimm
--
Wei Yang
Help you, Help me
On Wed, Jun 05, 2019 at 02:58:04PM -0700, Dan Williams wrote:
>Sub-section hotplug support reduces the unit of operation of hotplug
>from section-sized-units (PAGES_PER_SECTION) to sub-section-sized units
>(PAGES_PER_SUBSECTION). Teach shrink_{zone,pgdat}_span() to consider
>PAGES_PER_SUBSECTION boundaries as the points where pfn_valid(), not
>valid_section(), can toggle.
>
>Cc: Michal Hocko <[email protected]>
>Cc: Vlastimil Babka <[email protected]>
>Cc: Logan Gunthorpe <[email protected]>
>Reviewed-by: Pavel Tatashin <[email protected]>
>Reviewed-by: Oscar Salvador <[email protected]>
>Signed-off-by: Dan Williams <[email protected]>
>---
> mm/memory_hotplug.c | 29 ++++++++---------------------
> 1 file changed, 8 insertions(+), 21 deletions(-)
>
>diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>index 7b963c2d3a0d..647859a1d119 100644
>--- a/mm/memory_hotplug.c
>+++ b/mm/memory_hotplug.c
>@@ -318,12 +318,8 @@ static unsigned long find_smallest_section_pfn(int nid, struct zone *zone,
> unsigned long start_pfn,
> unsigned long end_pfn)
> {
>- struct mem_section *ms;
>-
>- for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SECTION) {
>- ms = __pfn_to_section(start_pfn);
>-
>- if (unlikely(!valid_section(ms)))
>+ for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SUBSECTION) {
>+ if (unlikely(!pfn_valid(start_pfn)))
> continue;
Hmm, we change the granularity of valid section from SECTION to SUBSECTION.
But we didn't change the granularity of node id and zone information.
For example, we found the node id of a pfn mismatch, we can skip the whole
section instead of a subsection.
Maybe this is not a big deal.
>
> if (unlikely(pfn_to_nid(start_pfn) != nid))
>@@ -343,15 +339,12 @@ static unsigned long find_biggest_section_pfn(int nid, struct zone *zone,
> unsigned long start_pfn,
> unsigned long end_pfn)
> {
>- struct mem_section *ms;
> unsigned long pfn;
>
> /* pfn is the end pfn of a memory section. */
> pfn = end_pfn - 1;
>- for (; pfn >= start_pfn; pfn -= PAGES_PER_SECTION) {
>- ms = __pfn_to_section(pfn);
>-
>- if (unlikely(!valid_section(ms)))
>+ for (; pfn >= start_pfn; pfn -= PAGES_PER_SUBSECTION) {
>+ if (unlikely(!pfn_valid(pfn)))
> continue;
>
> if (unlikely(pfn_to_nid(pfn) != nid))
>@@ -373,7 +366,6 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
> unsigned long z = zone_end_pfn(zone); /* zone_end_pfn namespace clash */
> unsigned long zone_end_pfn = z;
> unsigned long pfn;
>- struct mem_section *ms;
> int nid = zone_to_nid(zone);
>
> zone_span_writelock(zone);
>@@ -410,10 +402,8 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
> * it check the zone has only hole or not.
> */
> pfn = zone_start_pfn;
>- for (; pfn < zone_end_pfn; pfn += PAGES_PER_SECTION) {
>- ms = __pfn_to_section(pfn);
>-
>- if (unlikely(!valid_section(ms)))
>+ for (; pfn < zone_end_pfn; pfn += PAGES_PER_SUBSECTION) {
>+ if (unlikely(!pfn_valid(pfn)))
> continue;
>
> if (page_zone(pfn_to_page(pfn)) != zone)
>@@ -441,7 +431,6 @@ static void shrink_pgdat_span(struct pglist_data *pgdat,
> unsigned long p = pgdat_end_pfn(pgdat); /* pgdat_end_pfn namespace clash */
> unsigned long pgdat_end_pfn = p;
> unsigned long pfn;
>- struct mem_section *ms;
> int nid = pgdat->node_id;
>
> if (pgdat_start_pfn == start_pfn) {
>@@ -478,10 +467,8 @@ static void shrink_pgdat_span(struct pglist_data *pgdat,
> * has only hole or not.
> */
> pfn = pgdat_start_pfn;
>- for (; pfn < pgdat_end_pfn; pfn += PAGES_PER_SECTION) {
>- ms = __pfn_to_section(pfn);
>-
>- if (unlikely(!valid_section(ms)))
>+ for (; pfn < pgdat_end_pfn; pfn += PAGES_PER_SUBSECTION) {
>+ if (unlikely(!pfn_valid(pfn)))
> continue;
>
> if (pfn_to_nid(pfn) != nid)
>
>_______________________________________________
>Linux-nvdimm mailing list
>[email protected]
>https://lists.01.org/mailman/listinfo/linux-nvdimm
--
Wei Yang
Help you, Help me
On Wed, Jun 05, 2019 at 02:58:32PM -0700, Dan Williams wrote:
>Given there are no more usages of is_dev_zone() outside of 'ifdef
>CONFIG_ZONE_DEVICE' protection, kill off the compilation helper.
>
>Cc: Michal Hocko <[email protected]>
>Cc: Logan Gunthorpe <[email protected]>
>Acked-by: David Hildenbrand <[email protected]>
>Reviewed-by: Oscar Salvador <[email protected]>
>Reviewed-by: Pavel Tatashin <[email protected]>
>Signed-off-by: Dan Williams <[email protected]>
Reviewed-by: Wei Yang <[email protected]>
>---
> include/linux/mmzone.h | 12 ------------
> mm/page_alloc.c | 2 +-
> 2 files changed, 1 insertion(+), 13 deletions(-)
>
>diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>index 6dd52d544857..49e7fb452dfd 100644
>--- a/include/linux/mmzone.h
>+++ b/include/linux/mmzone.h
>@@ -855,18 +855,6 @@ static inline int local_memory_node(int node_id) { return node_id; };
> */
> #define zone_idx(zone) ((zone) - (zone)->zone_pgdat->node_zones)
>
>-#ifdef CONFIG_ZONE_DEVICE
>-static inline bool is_dev_zone(const struct zone *zone)
>-{
>- return zone_idx(zone) == ZONE_DEVICE;
>-}
>-#else
>-static inline bool is_dev_zone(const struct zone *zone)
>-{
>- return false;
>-}
>-#endif
>-
> /*
> * Returns true if a zone has pages managed by the buddy allocator.
> * All the reclaim decisions have to use this function rather than
>diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>index bd773efe5b82..5dff3f49a372 100644
>--- a/mm/page_alloc.c
>+++ b/mm/page_alloc.c
>@@ -5865,7 +5865,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
> unsigned long start = jiffies;
> int nid = pgdat->node_id;
>
>- if (WARN_ON_ONCE(!pgmap || !is_dev_zone(zone)))
>+ if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
> return;
>
> /*
>
>_______________________________________________
>Linux-nvdimm mailing list
>[email protected]
>https://lists.01.org/mailman/listinfo/linux-nvdimm
--
Wei Yang
Help you, Help me
On Sun, Jun 16, 2019 at 6:11 AM Wei Yang <[email protected]> wrote:
>
> On Wed, Jun 05, 2019 at 02:57:54PM -0700, Dan Williams wrote:
> >Towards enabling memory hotplug to track partial population of a
> >section, introduce 'struct mem_section_usage'.
> >
> >A pointer to a 'struct mem_section_usage' instance replaces the existing
> >pointer to a 'pageblock_flags' bitmap. Effectively it adds one more
> >'unsigned long' beyond the 'pageblock_flags' (usemap) allocation to
> >house a new 'subsection_map' bitmap. The new bitmap enables the memory
> >hot{plug,remove} implementation to act on incremental sub-divisions of a
> >section.
> >
> >The default SUBSECTION_SHIFT is chosen to keep the 'subsection_map' no
> >larger than a single 'unsigned long' on the major architectures.
> >Alternatively an architecture can define ARCH_SUBSECTION_SHIFT to
> >override the default PMD_SHIFT. Note that PowerPC needs to use
> >ARCH_SUBSECTION_SHIFT to workaround PMD_SHIFT being a non-constant
> >expression on PowerPC.
> >
> >The primary motivation for this functionality is to support platforms
> >that mix "System RAM" and "Persistent Memory" within a single section,
> >or multiple PMEM ranges with different mapping lifetimes within a single
> >section. The section restriction for hotplug has caused an ongoing saga
> >of hacks and bugs for devm_memremap_pages() users.
> >
> >Beyond the fixups to teach existing paths how to retrieve the 'usemap'
> >from a section, and updates to usemap allocation path, there are no
> >expected behavior changes.
> >
> >Cc: Michal Hocko <[email protected]>
> >Cc: Vlastimil Babka <[email protected]>
> >Cc: Logan Gunthorpe <[email protected]>
> >Cc: Oscar Salvador <[email protected]>
> >Cc: Pavel Tatashin <[email protected]>
> >Cc: Benjamin Herrenschmidt <[email protected]>
> >Cc: Paul Mackerras <[email protected]>
> >Cc: Michael Ellerman <[email protected]>
> >Signed-off-by: Dan Williams <[email protected]>
> >---
> > arch/powerpc/include/asm/sparsemem.h | 3 +
> > include/linux/mmzone.h | 48 +++++++++++++++++++-
> > mm/memory_hotplug.c | 18 ++++----
> > mm/page_alloc.c | 2 -
> > mm/sparse.c | 81 +++++++++++++++++-----------------
> > 5 files changed, 99 insertions(+), 53 deletions(-)
> >
> >diff --git a/arch/powerpc/include/asm/sparsemem.h b/arch/powerpc/include/asm/sparsemem.h
> >index 3192d454a733..1aa3c9303bf8 100644
> >--- a/arch/powerpc/include/asm/sparsemem.h
> >+++ b/arch/powerpc/include/asm/sparsemem.h
> >@@ -10,6 +10,9 @@
> > */
> > #define SECTION_SIZE_BITS 24
> >
> >+/* Reflect the largest possible PMD-size as the subsection-size constant */
> >+#define ARCH_SUBSECTION_SHIFT 24
> >+
> > #endif /* CONFIG_SPARSEMEM */
> >
> > #ifdef CONFIG_MEMORY_HOTPLUG
> >diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> >index 427b79c39b3c..ac163f2f274f 100644
> >--- a/include/linux/mmzone.h
> >+++ b/include/linux/mmzone.h
> >@@ -1161,6 +1161,44 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
> > #define SECTION_ALIGN_UP(pfn) (((pfn) + PAGES_PER_SECTION - 1) & PAGE_SECTION_MASK)
> > #define SECTION_ALIGN_DOWN(pfn) ((pfn) & PAGE_SECTION_MASK)
> >
> >+/*
> >+ * SUBSECTION_SHIFT must be constant since it is used to declare
> >+ * subsection_map and related bitmaps without triggering the generation
> >+ * of variable-length arrays. The most natural size for a subsection is
> >+ * a PMD-page. For architectures that do not have a constant PMD-size
> >+ * ARCH_SUBSECTION_SHIFT can be set to a constant max size, or otherwise
> >+ * fallback to 2MB.
> >+ */
> >+#if defined(ARCH_SUBSECTION_SHIFT)
> >+#define SUBSECTION_SHIFT (ARCH_SUBSECTION_SHIFT)
> >+#elif defined(PMD_SHIFT)
> >+#define SUBSECTION_SHIFT (PMD_SHIFT)
> >+#else
> >+/*
> >+ * Memory hotplug enabled platforms avoid this default because they
> >+ * either define ARCH_SUBSECTION_SHIFT, or PMD_SHIFT is a constant, but
> >+ * this is kept as a backstop to allow compilation on
> >+ * !ARCH_ENABLE_MEMORY_HOTPLUG archs.
> >+ */
> >+#define SUBSECTION_SHIFT 21
> >+#endif
> >+
> >+#define PFN_SUBSECTION_SHIFT (SUBSECTION_SHIFT - PAGE_SHIFT)
> >+#define PAGES_PER_SUBSECTION (1UL << PFN_SUBSECTION_SHIFT)
> >+#define PAGE_SUBSECTION_MASK ((~(PAGES_PER_SUBSECTION-1)))
>
> One pair of brackets could be removed, IMHO.
Sure.
>
> >+
> >+#if SUBSECTION_SHIFT > SECTION_SIZE_BITS
> >+#error Subsection size exceeds section size
> >+#else
> >+#define SUBSECTIONS_PER_SECTION (1UL << (SECTION_SIZE_BITS - SUBSECTION_SHIFT))
> >+#endif
> >+
> >+struct mem_section_usage {
> >+ DECLARE_BITMAP(subsection_map, SUBSECTIONS_PER_SECTION);
> >+ /* See declaration of similar field in struct zone */
> >+ unsigned long pageblock_flags[0];
> >+};
> >+
> > struct page;
> > struct page_ext;
> > struct mem_section {
> >@@ -1178,8 +1216,7 @@ struct mem_section {
> > */
> > unsigned long section_mem_map;
> >
> >- /* See declaration of similar field in struct zone */
> >- unsigned long *pageblock_flags;
> >+ struct mem_section_usage *usage;
> > #ifdef CONFIG_PAGE_EXTENSION
> > /*
> > * If SPARSEMEM, pgdat doesn't have page_ext pointer. We use
> >@@ -1210,6 +1247,11 @@ extern struct mem_section **mem_section;
> > extern struct mem_section mem_section[NR_SECTION_ROOTS][SECTIONS_PER_ROOT];
> > #endif
> >
> >+static inline unsigned long *section_to_usemap(struct mem_section *ms)
> >+{
> >+ return ms->usage->pageblock_flags;
>
> Do we need to consider the case when ms->usage is NULL?
No, this routine safely assumes it is always set.
On Tue, Jun 18, 2019 at 02:56:09PM -0700, Dan Williams wrote:
>On Sun, Jun 16, 2019 at 6:11 AM Wei Yang <[email protected]> wrote:
>>
>> On Wed, Jun 05, 2019 at 02:57:54PM -0700, Dan Williams wrote:
>> >Towards enabling memory hotplug to track partial population of a
>> >section, introduce 'struct mem_section_usage'.
>> >
>> >A pointer to a 'struct mem_section_usage' instance replaces the existing
>> >pointer to a 'pageblock_flags' bitmap. Effectively it adds one more
>> >'unsigned long' beyond the 'pageblock_flags' (usemap) allocation to
>> >house a new 'subsection_map' bitmap. The new bitmap enables the memory
>> >hot{plug,remove} implementation to act on incremental sub-divisions of a
>> >section.
>> >
>> >The default SUBSECTION_SHIFT is chosen to keep the 'subsection_map' no
>> >larger than a single 'unsigned long' on the major architectures.
>> >Alternatively an architecture can define ARCH_SUBSECTION_SHIFT to
>> >override the default PMD_SHIFT. Note that PowerPC needs to use
>> >ARCH_SUBSECTION_SHIFT to workaround PMD_SHIFT being a non-constant
>> >expression on PowerPC.
>> >
>> >The primary motivation for this functionality is to support platforms
>> >that mix "System RAM" and "Persistent Memory" within a single section,
>> >or multiple PMEM ranges with different mapping lifetimes within a single
>> >section. The section restriction for hotplug has caused an ongoing saga
>> >of hacks and bugs for devm_memremap_pages() users.
>> >
>> >Beyond the fixups to teach existing paths how to retrieve the 'usemap'
>> >from a section, and updates to usemap allocation path, there are no
>> >expected behavior changes.
>> >
>> >Cc: Michal Hocko <[email protected]>
>> >Cc: Vlastimil Babka <[email protected]>
>> >Cc: Logan Gunthorpe <[email protected]>
>> >Cc: Oscar Salvador <[email protected]>
>> >Cc: Pavel Tatashin <[email protected]>
>> >Cc: Benjamin Herrenschmidt <[email protected]>
>> >Cc: Paul Mackerras <[email protected]>
>> >Cc: Michael Ellerman <[email protected]>
>> >Signed-off-by: Dan Williams <[email protected]>
>> >---
>> > arch/powerpc/include/asm/sparsemem.h | 3 +
>> > include/linux/mmzone.h | 48 +++++++++++++++++++-
>> > mm/memory_hotplug.c | 18 ++++----
>> > mm/page_alloc.c | 2 -
>> > mm/sparse.c | 81 +++++++++++++++++-----------------
>> > 5 files changed, 99 insertions(+), 53 deletions(-)
>> >
>> >diff --git a/arch/powerpc/include/asm/sparsemem.h b/arch/powerpc/include/asm/sparsemem.h
>> >index 3192d454a733..1aa3c9303bf8 100644
>> >--- a/arch/powerpc/include/asm/sparsemem.h
>> >+++ b/arch/powerpc/include/asm/sparsemem.h
>> >@@ -10,6 +10,9 @@
>> > */
>> > #define SECTION_SIZE_BITS 24
>> >
>> >+/* Reflect the largest possible PMD-size as the subsection-size constant */
>> >+#define ARCH_SUBSECTION_SHIFT 24
>> >+
>> > #endif /* CONFIG_SPARSEMEM */
>> >
>> > #ifdef CONFIG_MEMORY_HOTPLUG
>> >diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> >index 427b79c39b3c..ac163f2f274f 100644
>> >--- a/include/linux/mmzone.h
>> >+++ b/include/linux/mmzone.h
>> >@@ -1161,6 +1161,44 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
>> > #define SECTION_ALIGN_UP(pfn) (((pfn) + PAGES_PER_SECTION - 1) & PAGE_SECTION_MASK)
>> > #define SECTION_ALIGN_DOWN(pfn) ((pfn) & PAGE_SECTION_MASK)
>> >
>> >+/*
>> >+ * SUBSECTION_SHIFT must be constant since it is used to declare
>> >+ * subsection_map and related bitmaps without triggering the generation
>> >+ * of variable-length arrays. The most natural size for a subsection is
>> >+ * a PMD-page. For architectures that do not have a constant PMD-size
>> >+ * ARCH_SUBSECTION_SHIFT can be set to a constant max size, or otherwise
>> >+ * fallback to 2MB.
>> >+ */
>> >+#if defined(ARCH_SUBSECTION_SHIFT)
>> >+#define SUBSECTION_SHIFT (ARCH_SUBSECTION_SHIFT)
>> >+#elif defined(PMD_SHIFT)
>> >+#define SUBSECTION_SHIFT (PMD_SHIFT)
>> >+#else
>> >+/*
>> >+ * Memory hotplug enabled platforms avoid this default because they
>> >+ * either define ARCH_SUBSECTION_SHIFT, or PMD_SHIFT is a constant, but
>> >+ * this is kept as a backstop to allow compilation on
>> >+ * !ARCH_ENABLE_MEMORY_HOTPLUG archs.
>> >+ */
>> >+#define SUBSECTION_SHIFT 21
>> >+#endif
>> >+
>> >+#define PFN_SUBSECTION_SHIFT (SUBSECTION_SHIFT - PAGE_SHIFT)
>> >+#define PAGES_PER_SUBSECTION (1UL << PFN_SUBSECTION_SHIFT)
>> >+#define PAGE_SUBSECTION_MASK ((~(PAGES_PER_SUBSECTION-1)))
>>
>> One pair of brackets could be removed, IMHO.
>
>Sure.
>
>>
>> >+
>> >+#if SUBSECTION_SHIFT > SECTION_SIZE_BITS
>> >+#error Subsection size exceeds section size
>> >+#else
>> >+#define SUBSECTIONS_PER_SECTION (1UL << (SECTION_SIZE_BITS - SUBSECTION_SHIFT))
>> >+#endif
>> >+
>> >+struct mem_section_usage {
>> >+ DECLARE_BITMAP(subsection_map, SUBSECTIONS_PER_SECTION);
>> >+ /* See declaration of similar field in struct zone */
>> >+ unsigned long pageblock_flags[0];
>> >+};
>> >+
>> > struct page;
>> > struct page_ext;
>> > struct mem_section {
>> >@@ -1178,8 +1216,7 @@ struct mem_section {
>> > */
>> > unsigned long section_mem_map;
>> >
>> >- /* See declaration of similar field in struct zone */
>> >- unsigned long *pageblock_flags;
>> >+ struct mem_section_usage *usage;
>> > #ifdef CONFIG_PAGE_EXTENSION
>> > /*
>> > * If SPARSEMEM, pgdat doesn't have page_ext pointer. We use
>> >@@ -1210,6 +1247,11 @@ extern struct mem_section **mem_section;
>> > extern struct mem_section mem_section[NR_SECTION_ROOTS][SECTIONS_PER_ROOT];
>> > #endif
>> >
>> >+static inline unsigned long *section_to_usemap(struct mem_section *ms)
>> >+{
>> >+ return ms->usage->pageblock_flags;
>>
>> Do we need to consider the case when ms->usage is NULL?
>
>No, this routine safely assumes it is always set.
Then everything looks good to me.
Reviewed-by: Wei Yang <[email protected]>
>_______________________________________________
>Linux-nvdimm mailing list
>[email protected]
>https://lists.01.org/mailman/listinfo/linux-nvdimm
--
Wei Yang
Help you, Help me
On Mon, Jun 17, 2019 at 3:32 PM Dan Williams <[email protected]> wrote:
>
> On Mon, Jun 17, 2019 at 3:22 PM Wei Yang <[email protected]> wrote:
> >
> > On Wed, Jun 05, 2019 at 02:57:59PM -0700, Dan Williams wrote:
> > >Prepare for hot{plug,remove} of sub-ranges of a section by tracking a
> > >sub-section active bitmask, each bit representing a PMD_SIZE span of the
> > >architecture's memory hotplug section size.
> > >
> > >The implications of a partially populated section is that pfn_valid()
> > >needs to go beyond a valid_section() check and read the sub-section
> > >active ranges from the bitmask. The expectation is that the bitmask
> > >(subsection_map) fits in the same cacheline as the valid_section() data,
> > >so the incremental performance overhead to pfn_valid() should be
> > >negligible.
> > >
> > >Cc: Michal Hocko <[email protected]>
> > >Cc: Vlastimil Babka <[email protected]>
> > >Cc: Logan Gunthorpe <[email protected]>
> > >Cc: Oscar Salvador <[email protected]>
> > >Cc: Pavel Tatashin <[email protected]>
> > >Tested-by: Jane Chu <[email protected]>
> > >Signed-off-by: Dan Williams <[email protected]>
> > >---
> > > include/linux/mmzone.h | 29 ++++++++++++++++++++++++++++-
> > > mm/page_alloc.c | 4 +++-
> > > mm/sparse.c | 35 +++++++++++++++++++++++++++++++++++
> > > 3 files changed, 66 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > >index ac163f2f274f..6dd52d544857 100644
> > >--- a/include/linux/mmzone.h
> > >+++ b/include/linux/mmzone.h
> > >@@ -1199,6 +1199,8 @@ struct mem_section_usage {
> > > unsigned long pageblock_flags[0];
> > > };
> > >
> > >+void subsection_map_init(unsigned long pfn, unsigned long nr_pages);
> > >+
> > > struct page;
> > > struct page_ext;
> > > struct mem_section {
> > >@@ -1336,12 +1338,36 @@ static inline struct mem_section *__pfn_to_section(unsigned long pfn)
> > >
> > > extern int __highest_present_section_nr;
> > >
> > >+static inline int subsection_map_index(unsigned long pfn)
> > >+{
> > >+ return (pfn & ~(PAGE_SECTION_MASK)) / PAGES_PER_SUBSECTION;
> > >+}
> > >+
> > >+#ifdef CONFIG_SPARSEMEM_VMEMMAP
> > >+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> > >+{
> > >+ int idx = subsection_map_index(pfn);
> > >+
> > >+ return test_bit(idx, ms->usage->subsection_map);
> > >+}
> > >+#else
> > >+static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> > >+{
> > >+ return 1;
> > >+}
> > >+#endif
> > >+
> > > #ifndef CONFIG_HAVE_ARCH_PFN_VALID
> > > static inline int pfn_valid(unsigned long pfn)
> > > {
> > >+ struct mem_section *ms;
> > >+
> > > if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
> > > return 0;
> > >- return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
> > >+ ms = __nr_to_section(pfn_to_section_nr(pfn));
> > >+ if (!valid_section(ms))
> > >+ return 0;
> > >+ return pfn_section_valid(ms, pfn);
> > > }
> > > #endif
> > >
> > >@@ -1373,6 +1399,7 @@ void sparse_init(void);
> > > #define sparse_init() do {} while (0)
> > > #define sparse_index_init(_sec, _nid) do {} while (0)
> > > #define pfn_present pfn_valid
> > >+#define subsection_map_init(_pfn, _nr_pages) do {} while (0)
> > > #endif /* CONFIG_SPARSEMEM */
> > >
> > > /*
> > >diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > >index c6d8224d792e..bd773efe5b82 100644
> > >--- a/mm/page_alloc.c
> > >+++ b/mm/page_alloc.c
> > >@@ -7292,10 +7292,12 @@ void __init free_area_init_nodes(unsigned long *max_zone_pfn)
> > >
> > > /* Print out the early node map */
> > > pr_info("Early memory node ranges\n");
> > >- for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid)
> > >+ for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid) {
> > > pr_info(" node %3d: [mem %#018Lx-%#018Lx]\n", nid,
> > > (u64)start_pfn << PAGE_SHIFT,
> > > ((u64)end_pfn << PAGE_SHIFT) - 1);
> > >+ subsection_map_init(start_pfn, end_pfn - start_pfn);
> > >+ }
> >
> > Just curious about why we set subsection here?
> >
> > Function free_area_init_nodes() mostly handles pgdat, if I am correct. Setup
> > subsection here looks like touching some lower level system data structure.
>
> Correct, I'm not sure how it ended up there, but it was the source of
> a bug that was fixed with this change:
>
> https://lore.kernel.org/lkml/CAPcyv4hjvBPDYKpp2Gns3-cc2AQ0AVS1nLk-K3fwXeRUvvzQLg@mail.gmail.com/
On second thought I'm going to keep subsection_map_init() in
free_area_init_nodes(), but instead teach pfn_valid() to return true
for all "early" sections. There are code paths that use pfn_valid() as
a coarse check before validating against pgdat for real validity of
online memory. It is sufficient and safe for those to assume that all
early sections are fully pfn_valid, while ZONE_DEVICE hotplug can see
the more precise subsection_map.
On Mon, Jun 17, 2019 at 6:42 PM Wei Yang <[email protected]> wrote:
>
> On Wed, Jun 05, 2019 at 02:58:04PM -0700, Dan Williams wrote:
> >Sub-section hotplug support reduces the unit of operation of hotplug
> >from section-sized-units (PAGES_PER_SECTION) to sub-section-sized units
> >(PAGES_PER_SUBSECTION). Teach shrink_{zone,pgdat}_span() to consider
> >PAGES_PER_SUBSECTION boundaries as the points where pfn_valid(), not
> >valid_section(), can toggle.
> >
> >Cc: Michal Hocko <[email protected]>
> >Cc: Vlastimil Babka <[email protected]>
> >Cc: Logan Gunthorpe <[email protected]>
> >Reviewed-by: Pavel Tatashin <[email protected]>
> >Reviewed-by: Oscar Salvador <[email protected]>
> >Signed-off-by: Dan Williams <[email protected]>
> >---
> > mm/memory_hotplug.c | 29 ++++++++---------------------
> > 1 file changed, 8 insertions(+), 21 deletions(-)
> >
> >diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> >index 7b963c2d3a0d..647859a1d119 100644
> >--- a/mm/memory_hotplug.c
> >+++ b/mm/memory_hotplug.c
> >@@ -318,12 +318,8 @@ static unsigned long find_smallest_section_pfn(int nid, struct zone *zone,
> > unsigned long start_pfn,
> > unsigned long end_pfn)
> > {
> >- struct mem_section *ms;
> >-
> >- for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SECTION) {
> >- ms = __pfn_to_section(start_pfn);
> >-
> >- if (unlikely(!valid_section(ms)))
> >+ for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SUBSECTION) {
> >+ if (unlikely(!pfn_valid(start_pfn)))
> > continue;
>
> Hmm, we change the granularity of valid section from SECTION to SUBSECTION.
> But we didn't change the granularity of node id and zone information.
>
> For example, we found the node id of a pfn mismatch, we can skip the whole
> section instead of a subsection.
>
> Maybe this is not a big deal.
I don't see a problem.