From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A6C8FC4332F for ; Mon, 30 Oct 2023 02:48:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3AC666B017E; Sun, 29 Oct 2023 22:48:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 35CC16B017F; Sun, 29 Oct 2023 22:48:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 223BD6B0180; Sun, 29 Oct 2023 22:48:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 104B36B017E for ; Sun, 29 Oct 2023 22:48:20 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id D656140361 for ; Mon, 30 Oct 2023 02:48:19 +0000 (UTC) X-FDA: 81400593918.27.4140672 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.136]) by imf02.hostedemail.com (Postfix) with ESMTP id 4D1D080008 for ; Mon, 30 Oct 2023 02:48:16 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=Q8c3LEoP; spf=pass (imf02.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.136 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1698634098; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Ut6tr5MUpi5/hMSgwV8hXB3aezxKqD/jxxmdj2A1z8k=; b=CER3YmkRmYyFv5a3qqkGI+KuLnlE3Xs3l17CDHQVjR1D6j3M6srj9LQsYGWYBd4CBrJ+Kr VYTzurBP09jAty3UTb7bio5Ffj4t39PvYuBijLiTzSr81l8ozYpn5CvbkDAAs+mdIhEADz h1rvpQtAZYjzW1l4L8LTOpqGN0lxChw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1698634098; a=rsa-sha256; cv=none; b=V+evlaiwDHOP2JWd9tADtqJ/jb8mAeIRlf5s9YymtRorKbErgmWmSRD+w8vAIldJGDiR+U Z5luX0g+Ikw/Pwz7hXOJD+RV/tupQozJGzf2WmerORtL0ev2wpxTPQG9hGU5gp00tCRONM w4f4eopV6d4gsxxfYjjDlQ4jRoP4SQ8= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=Q8c3LEoP; spf=pass (imf02.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.136 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1698634096; x=1730170096; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=NcH0XnP6VoshU9abBq6Ta+z+6AM7rwOc9BoEsxOjEI8=; b=Q8c3LEoPZCnuYDMNJVwtj0klBT2wR0PEWMqxlP2FSPvwuzIm5oQaNDRm O9pw7/uxoa0FcR/up2yLS4r3lLTh/DDSC5H9TrelmgLx7WMdmhaHmg0rY dpYzv9boTo9Ct8hMQUygiuHFiuGSuoAAadMbUyOZgu/EdOXEOmQvWW2wl 7P5xox035swPE7uMJ9i2rAAPgGhGpqgdoNsEP6+dm6fe2IyRE4lvlI3U8 gixis1kPt3NYgyrWf7m64SVF/W0rpYCkhZE1h3B9ZUPbAczunfCYlXoRN nJVn2bdZjbLH1y9B7NNJHiUzDQkArXGncEkvgv6VPVdIydDfPhC83EbUx g==; X-IronPort-AV: E=McAfee;i="6600,9927,10878"; a="367347172" X-IronPort-AV: E=Sophos;i="6.03,262,1694761200"; d="scan'208";a="367347172" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Oct 2023 19:48:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10878"; a="760135503" X-IronPort-AV: E=Sophos;i="6.03,262,1694761200"; d="scan'208";a="760135503" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Oct 2023 19:48:10 -0700 From: "Huang, Ying" To: Vishal Verma Cc: Andrew Morton , David Hildenbrand , Oscar Salvador , Dan Williams , Dave Jiang , , , , , Dave Hansen , "Aneesh Kumar K.V" , Michal Hocko , Jonathan Cameron , Jeff Moyer Subject: Re: [PATCH v7 2/3] mm/memory_hotplug: split memmap_on_memory requests across memblocks In-Reply-To: <20231025-vv-kmem_memmap-v7-2-4a76d7652df5@intel.com> (Vishal Verma's message of "Wed, 25 Oct 2023 16:44:34 -0600") References: <20231025-vv-kmem_memmap-v7-0-4a76d7652df5@intel.com> <20231025-vv-kmem_memmap-v7-2-4a76d7652df5@intel.com> Date: Mon, 30 Oct 2023 10:46:08 +0800 Message-ID: <875y2oddzj.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Queue-Id: 4D1D080008 X-Rspam-User: X-Stat-Signature: fazux3nhgk1hchuu1e9ae4oa8uazg4xz X-Rspamd-Server: rspam03 X-HE-Tag: 1698634096-545236 X-HE-Meta: U2FsdGVkX1+x1ncwAnC02ybO4nmBMJxDslRyvw7jhtP8W9KdaePCOBC5FXci9iFFr46qqtKpNCYbSrzAc/tfFSdfpkmnIU+xRVmhH0LNsO6C8Qr0J8M4hRAK2/b4+d/lANN5S7Kud5CwHXyLeSH1XmFpeSv8h8IbaN3xNcy4DB7UF6ZF+ZJ7PmPQ/KrL1qD6q09sKlSOhmXDvb6Eia6jOR4SWlThGwNSUbolV0qx5dvBvn65OFxTXwcaXaj6iU4aRIFaLfALZ6LcdtY0HvlhP1LFowGFi5b0C0/wp6TsgLPZT26XZiWqxqnC4mr5uI9zPhD3YoUiEUyIuJvSTc74XNTZ8EB526FT9jWALVmW9YrsU4pgsJEl+ExFgwqUTXUMB8jIcRNHG8IuqNYZBeuzxSD3+7NIHt6VywFFk6upd5sjKbkIsmzAy3ogZ7RlMoaLvsfUfC9YjqhwYOE5kDJzUgSwORRzR7h05Jrc/IfkMdHzVQQk/wxne2fABLY+WJ47ngu+8ge1MC0GKOb9d5Lo2Fj5W3wsFcf7beAkcPaOaMycjxK/EToqfjBETMQELuLtheKmxckFZfSyyopBQSX8FM5ydtv47LfH6w1296WcSxFZI2BA8BSWqqVZRBpTGgZWIqFs9stHes7hmv5Zc+Bfy+usxEWfwpfQZcURQr2frTc3QTkvwQqVX2FRaQLnCiqOvHvi7YbaYwsTEK7OVCEQ29oi15wmHoojhwt54BItmUbKRT0GeBxHJOmYZYVAFUvsu5Y3ALo/jKERebfqDoXevAbSdqjlizYmJzxAdw0rX3CC4fhpfHFP3HNsyRItAC/r5+1UYgltNzntoetpTpduOV6owHaoHYVcq0xtTBXebLhAsjj4Bc5+eRTG4hxO7+ViqYCkTHLFXJ++2vb1dr3uF2dNZRWD9Miia+kfZjSXk25dCfW75QcXUENvFrhGKhXmDruofEOAZ2DKpjickJG /GPHvuyq iiTDBGLPzGE4KNijc9cC9DfCgHsYsFEgNua04flsj2bY7eQwOFZtpRUVRWhOxawOSYfQt9K1/xmUk3bJ7wG+HHcv9y+kksKdk6NZUa49uPgzehQpRMoBSrvmP/VA4+b3uX8tQG56jx55HTnSJnOzsrsQKOqgH/szzuFPHiRkfuIpu4ZX8Q7jhqIBOHDr88afI0TiAFHb2GaEjYBHVYPPsVBZQ5NQPUonomvlgp9RCK1qdU2Xr7+LNgWWNyQcv+adiAXIfdH1HhPC/Km0NY4S3U9T57XPO5r/JIG9M9ri5gc5mJekXwJrZCc3o/iiuXTfQKDliYRKALP8sXrqR1r1qWSwrki/IugvtyouKDMLF8F7uyRpuUBjzW+tg3hPSf6oS+QEpEvjOVMiwGF7HHdn6LSrsRJBrI+0e/HaGCznn8aEz/f8= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Vishal Verma writes: > The MHP_MEMMAP_ON_MEMORY flag for hotplugged memory is restricted to > 'memblock_size' chunks of memory being added. Adding a larger span of > memory precludes memmap_on_memory semantics. > > For users of hotplug such as kmem, large amounts of memory might get > added from the CXL subsystem. In some cases, this amount may exceed the > available 'main memory' to store the memmap for the memory being added. > In this case, it is useful to have a way to place the memmap on the > memory being added, even if it means splitting the addition into > memblock-sized chunks. > > Change add_memory_resource() to loop over memblock-sized chunks of > memory if caller requested memmap_on_memory, and if other conditions for > it are met. Teach try_remove_memory() to also expect that a memory > range being removed might have been split up into memblock sized chunks, > and to loop through those as needed. > > This does preclude being able to use PUD mappings in the direct map; a > proposal to how this could be optimized in the future is laid out > here[1]. > > [1]: https://lore.kernel.org/linux-mm/b6753402-2de9-25b2-36e9-eacd49752b19@redhat.com/ > > Cc: Andrew Morton > Cc: David Hildenbrand > Cc: Michal Hocko > Cc: Oscar Salvador > Cc: Dan Williams > Cc: Dave Jiang > Cc: Dave Hansen > Cc: Huang Ying > Suggested-by: David Hildenbrand > Reviewed-by: Dan Williams > Signed-off-by: Vishal Verma > --- > mm/memory_hotplug.c | 209 ++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 144 insertions(+), 65 deletions(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 6be7de9efa55..b97035193090 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -1380,6 +1380,48 @@ static bool mhp_supports_memmap_on_memory(unsigned long size) > return arch_supports_memmap_on_memory(vmemmap_size); > } > > +static int create_altmaps_and_memory_blocks(int nid, struct memory_group *group, > + u64 start, u64 size) > +{ > + unsigned long memblock_size = memory_block_size_bytes(); > + u64 cur_start; > + int ret; > + > + for (cur_start = start; cur_start < start + size; > + cur_start += memblock_size) { > + struct mhp_params params = { .pgprot = > + pgprot_mhp(PAGE_KERNEL) }; > + struct vmem_altmap mhp_altmap = { > + .base_pfn = PHYS_PFN(cur_start), > + .end_pfn = PHYS_PFN(cur_start + memblock_size - 1), > + }; > + > + mhp_altmap.free = memory_block_memmap_on_memory_pages(); > + params.altmap = kmemdup(&mhp_altmap, sizeof(struct vmem_altmap), > + GFP_KERNEL); > + if (!params.altmap) > + return -ENOMEM; > + > + /* call arch's memory hotadd */ > + ret = arch_add_memory(nid, cur_start, memblock_size, ¶ms); > + if (ret < 0) { > + kfree(params.altmap); Should we call remove_memory_blocks_and_altmaps(start, cur_start - start); here to clean up resources? -- Best Regards, Huang, Ying > + return ret; > + } > + > + /* create memory block devices after memory was added */ > + ret = create_memory_block_devices(cur_start, memblock_size, > + params.altmap, group); > + if (ret) { > + arch_remove_memory(cur_start, memblock_size, NULL); > + kfree(params.altmap); > + return ret; > + } > + } > + > + return 0; > +} > + > /* > * NOTE: The caller must call lock_device_hotplug() to serialize hotplug > * and online/offline operations (triggered e.g. by sysfs). > @@ -1390,10 +1432,6 @@ int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags) > { > struct mhp_params params = { .pgprot = pgprot_mhp(PAGE_KERNEL) }; > enum memblock_flags memblock_flags = MEMBLOCK_NONE; > - struct vmem_altmap mhp_altmap = { > - .base_pfn = PHYS_PFN(res->start), > - .end_pfn = PHYS_PFN(res->end), > - }; > struct memory_group *group = NULL; > u64 start, size; > bool new_node = false; > @@ -1436,28 +1474,22 @@ int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags) > /* > * Self hosted memmap array > */ > - if (mhp_flags & MHP_MEMMAP_ON_MEMORY) { > - if (mhp_supports_memmap_on_memory(size)) { > - mhp_altmap.free = memory_block_memmap_on_memory_pages(); > - params.altmap = kmemdup(&mhp_altmap, > - sizeof(struct vmem_altmap), > - GFP_KERNEL); > - if (!params.altmap) > - goto error; > + if ((mhp_flags & MHP_MEMMAP_ON_MEMORY) && > + mhp_supports_memmap_on_memory(memory_block_size_bytes())) { > + ret = create_altmaps_and_memory_blocks(nid, group, start, size); > + if (ret) > + goto error; > + } else { > + ret = arch_add_memory(nid, start, size, ¶ms); > + if (ret < 0) > + goto error; > + > + /* create memory block devices after memory was added */ > + ret = create_memory_block_devices(start, size, NULL, group); > + if (ret) { > + arch_remove_memory(start, size, NULL); > + goto error; > } > - /* fallback to not using altmap */ > - } > - > - /* call arch's memory hotadd */ > - ret = arch_add_memory(nid, start, size, ¶ms); > - if (ret < 0) > - goto error_free; > - > - /* create memory block devices after memory was added */ > - ret = create_memory_block_devices(start, size, params.altmap, group); > - if (ret) { > - arch_remove_memory(start, size, NULL); > - goto error_free; > } > > if (new_node) { > @@ -1494,8 +1526,6 @@ int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags) > walk_memory_blocks(start, size, NULL, online_memory_block); > > return ret; > -error_free: > - kfree(params.altmap); > error: > if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK)) > memblock_remove(start, size); > @@ -2076,6 +2106,16 @@ static int test_has_altmap_cb(struct memory_block *mem, void *arg) > return 0; > } > > +static int count_memory_range_altmaps_cb(struct memory_block *mem, void *arg) > +{ > + u64 *num_altmaps = (u64 *)arg; > + > + if (mem->altmap) > + *num_altmaps += 1; > + > + return 0; > +} > + > static int check_cpu_on_node(int nid) > { > int cpu; > @@ -2146,11 +2186,69 @@ void try_offline_node(int nid) > } > EXPORT_SYMBOL(try_offline_node); > > -static int __ref try_remove_memory(u64 start, u64 size) > +static void __ref remove_memory_blocks_and_altmaps(u64 start, u64 size) > { > - struct memory_block *mem; > - int rc = 0, nid = NUMA_NO_NODE; > + unsigned long memblock_size = memory_block_size_bytes(); > struct vmem_altmap *altmap = NULL; > + struct memory_block *mem; > + u64 cur_start; > + int rc; > + > + /* > + * For memmap_on_memory, the altmaps could have been added on > + * a per-memblock basis. Loop through the entire range if so, > + * and remove each memblock and its altmap. > + */ > + for (cur_start = start; cur_start < start + size; > + cur_start += memblock_size) { > + rc = walk_memory_blocks(cur_start, memblock_size, &mem, > + test_has_altmap_cb); > + if (rc) { > + altmap = mem->altmap; > + /* > + * Mark altmap NULL so that we can add a debug > + * check on memblock free. > + */ > + mem->altmap = NULL; > + } > + > + remove_memory_block_devices(cur_start, memblock_size); > + > + arch_remove_memory(cur_start, memblock_size, altmap); > + > + /* Verify that all vmemmap pages have actually been freed. */ > + if (altmap) { > + WARN(altmap->alloc, "Altmap not fully unmapped"); > + kfree(altmap); > + } > + } > +} > + > +static int memory_blocks_have_altmaps(u64 start, u64 size) > +{ > + u64 num_memblocks = size / memory_block_size_bytes(); > + u64 num_altmaps = 0; > + > + if (!mhp_memmap_on_memory()) > + return 0; > + > + walk_memory_blocks(start, size, &num_altmaps, > + count_memory_range_altmaps_cb); > + > + if (num_altmaps == 0) > + return 0; > + > + if (num_memblocks != num_altmaps) { > + WARN_ONCE(1, "Not all memblocks in range have altmaps"); > + return -EINVAL; > + } > + > + return 1; > +} > + > +static int __ref try_remove_memory(u64 start, u64 size) > +{ > + int rc, nid = NUMA_NO_NODE; > > BUG_ON(check_hotplug_memory_range(start, size)); > > @@ -2167,45 +2265,25 @@ static int __ref try_remove_memory(u64 start, u64 size) > if (rc) > return rc; > > - /* > - * We only support removing memory added with MHP_MEMMAP_ON_MEMORY in > - * the same granularity it was added - a single memory block. > - */ > - if (mhp_memmap_on_memory()) { > - rc = walk_memory_blocks(start, size, &mem, test_has_altmap_cb); > - if (rc) { > - if (size != memory_block_size_bytes()) { > - pr_warn("Refuse to remove %#llx - %#llx," > - "wrong granularity\n", > - start, start + size); > - return -EINVAL; > - } > - altmap = mem->altmap; > - /* > - * Mark altmap NULL so that we can add a debug > - * check on memblock free. > - */ > - mem->altmap = NULL; > - } > - } > - > /* remove memmap entry */ > firmware_map_remove(start, start + size, "System RAM"); > > - /* > - * Memory block device removal under the device_hotplug_lock is > - * a barrier against racing online attempts. > - */ > - remove_memory_block_devices(start, size); > - > mem_hotplug_begin(); > > - arch_remove_memory(start, size, altmap); > - > - /* Verify that all vmemmap pages have actually been freed. */ > - if (altmap) { > - WARN(altmap->alloc, "Altmap not fully unmapped"); > - kfree(altmap); > + rc = memory_blocks_have_altmaps(start, size); > + if (rc < 0) { > + goto err; > + } else if (rc == 0) { > + /* > + * Memory block device removal under the device_hotplug_lock is > + * a barrier against racing online attempts. > + * No altmaps present, do the removal directly > + */ > + remove_memory_block_devices(start, size); > + arch_remove_memory(start, size, NULL); > + } else { > + /* all memblocks in the range have altmaps */ > + remove_memory_blocks_and_altmaps(start, size); > } > > if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK)) { > @@ -2218,8 +2296,9 @@ static int __ref try_remove_memory(u64 start, u64 size) > if (nid != NUMA_NO_NODE) > try_offline_node(nid); > > +err: > mem_hotplug_done(); > - return 0; > + return (rc < 0 ? rc : 0); > } > > /**