lkml.org 
[lkml]   [2020]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 2/3] mm: don't rely on system state to detect hot-plug operations
    From
    Date
    Le 14/09/2020 à 19:15, David Hildenbrand a écrit :
    >> arch/ia64/mm/init.c | 4 +--
    >> drivers/base/node.c | 86 ++++++++++++++++++++++++++++----------------
    >> include/linux/node.h | 11 +++---
    >> mm/memory_hotplug.c | 5 +--
    >> 4 files changed, 68 insertions(+), 38 deletions(-)
    >>
    >> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
    >> index b5054b5e77c8..8e7b8c6c576e 100644
    >> --- a/arch/ia64/mm/init.c
    >> +++ b/arch/ia64/mm/init.c
    >> @@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
    >> if (map_start < map_end)
    >> memmap_init_zone((unsigned long)(map_end - map_start),
    >> args->nid, args->zone, page_to_pfn(map_start),
    >> - MEMPLUG_EARLY, NULL);
    >> + MEMINIT_EARLY, NULL);
    >
    > Patch #1.

    Sure, this explains why I was able to build on ia64 but that's not the right place.

    >> return 0;
    >> }
    >>
    >> @@ -548,7 +548,7 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
    >> {
    >> if (!vmem_map) {
    >> memmap_init_zone(size, nid, zone, start_pfn,
    >> - MEMPLUG_EARLY, NULL);
    >> + MEMINIT_EARLY, NULL);

    I'll fix that too.

    >> } else {
    >> struct page *start;
    >> struct memmap_init_callback_data args;
    >> diff --git a/drivers/base/node.c b/drivers/base/node.c
    >> index 508b80f6329b..01ee73c9d675 100644
    >> --- a/drivers/base/node.c
    >> +++ b/drivers/base/node.c
    >> @@ -761,14 +761,36 @@ static int __ref get_nid_for_pfn(unsigned long pfn)
    >> return pfn_to_nid(pfn);
    >> }
    >>
    >> +static int do_register_memory_block_under_node(int nid,
    >> + struct memory_block *mem_blk)
    >> +{
    >> + int ret;
    >> +
    >> + /*
    >> + * If this memory block spans multiple nodes, we only indicate
    >> + * the last processed node.
    >> + */
    >> + mem_blk->nid = nid;
    >> +
    >> + ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
    >> + &mem_blk->dev.kobj,
    >> + kobject_name(&mem_blk->dev.kobj));
    >> + if (ret)
    >> + return ret;
    >> +
    >> + return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
    >> + &node_devices[nid]->dev.kobj,
    >> + kobject_name(&node_devices[nid]->dev.kobj));
    >> +}
    >> +
    >> /* register memory section under specified node if it spans that node */
    >> -static int register_mem_sect_under_node(struct memory_block *mem_blk,
    >> - void *arg)
    >> +static int register_mem_block_under_node_early(struct memory_block *mem_blk,
    >> + void *arg)
    >> {
    >> unsigned long memory_block_pfns = memory_block_size_bytes() / PAGE_SIZE;
    >> unsigned long start_pfn = section_nr_to_pfn(mem_blk->start_section_nr);
    >> unsigned long end_pfn = start_pfn + memory_block_pfns - 1;
    >> - int ret, nid = *(int *)arg;
    >> + int nid = *(int *)arg;
    >> unsigned long pfn;
    >>
    >> for (pfn = start_pfn; pfn <= end_pfn; pfn++) {
    >> @@ -785,38 +807,34 @@ static int register_mem_sect_under_node(struct memory_block *mem_blk,
    >> }
    >>
    >> /*
    >> - * We need to check if page belongs to nid only for the boot
    >> - * case, during hotplug we know that all pages in the memory
    >> - * block belong to the same node.
    >> - */
    >> - if (system_state == SYSTEM_BOOTING) {
    >> - page_nid = get_nid_for_pfn(pfn);
    >> - if (page_nid < 0)
    >> - continue;
    >> - if (page_nid != nid)
    >> - continue;
    >> - }
    >> -
    >> - /*
    >> - * If this memory block spans multiple nodes, we only indicate
    >> - * the last processed node.
    >> + * We need to check if page belongs to nid only at the boot
    >> + * case because node's ranges can be interleaved.
    >> */
    >> - mem_blk->nid = nid;
    >> -
    >> - ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
    >> - &mem_blk->dev.kobj,
    >> - kobject_name(&mem_blk->dev.kobj));
    >> - if (ret)
    >> - return ret;
    >> + page_nid = get_nid_for_pfn(pfn);
    >> + if (page_nid < 0)
    >> + continue;
    >> + if (page_nid != nid)
    >> + continue;
    >>
    >> - return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
    >> - &node_devices[nid]->dev.kobj,
    >> - kobject_name(&node_devices[nid]->dev.kobj));
    >> + /* The memory block is registered to the first matching node */
    >
    > That comment is misleading in that context.
    >
    > A memory block is registered if there is at least a page that belongs to
    > the nid. It's perfectly fine to have a single memory block belong to
    > multiple NUMA nodes (when the split is within a memory block). I'd just
    > drop it.

    I agree the comment is not accurate, I'll drop it.

    \
     
     \ /
      Last update: 2020-09-15 09:09    [W:4.874 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site