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 6B407E95A61 for ; Sat, 7 Oct 2023 08:57:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E8B006B02E8; Sat, 7 Oct 2023 04:57:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E3ADF6B02E9; Sat, 7 Oct 2023 04:57:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D29616B02EA; Sat, 7 Oct 2023 04:57:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id C18D86B02E8 for ; Sat, 7 Oct 2023 04:57:42 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 7D5A9B42CF for ; Sat, 7 Oct 2023 08:57:32 +0000 (UTC) X-FDA: 81318061944.20.1F033F6 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.43]) by imf29.hostedemail.com (Postfix) with ESMTP id C50FE120003 for ; Sat, 7 Oct 2023 08:57:29 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=nY8NJMDA; spf=pass (imf29.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.43 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696669050; a=rsa-sha256; cv=none; b=KcPTivMb951dkjbT3eLJastlOhU3ZoxR/Slqo8qAkvxggBkQxBuW8Qqla3S1Cp9kSvYBob ajJUslRR4dJUl0eN8tx+NrDr60iCAWy589QreKPYCYDcQ2pNdquVNSbuSm4xQULpQK4Lox b0F0+Xtw+y33yEJGmk35C4BzeMj1TZg= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=nY8NJMDA; spf=pass (imf29.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.43 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=1696669050; 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=4sEERAD4G1d4mZ3o9aJwoZxGfoOA0NVleD4UgwYbEdc=; b=EkxAkQmrFXEGZZ6iUb9nBDTvszaKDO+TJYmI4pBQzxzkHsLfuRCgXIqY11ve8n4zAE6kfU oF05IvMzDJ8lQf1AAeSbfJtcJZNTWcJzU6Csh9f9oRJJ/f5zIuHRxA7y79iqiHkF+k1Tfy ahGdpqUWPp8yTs10Vh0n2ndHtJ/fdM8= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1696669049; x=1728205049; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=zZfpmpOa9xub7U4t7n92Ku/R19iFPsHPDUZaIQJfc8A=; b=nY8NJMDAaFeDoU4KDIF/I/GgxFG0lkUbYbK1kG1pkHtUSh5uhbVPhmAs w8ngyy6JQwN5yhICfmSyvoOq/ggqQVevXAEg5AbgquKhexK4BWR8uUU82 XAvIMFu8q6Pnt3coJ6MzWzNODekdcVT77KEsdmylc+TOhf+3LX71RCfWW ckodELDHLJoUPERBG8tf7RoXXzZlfo+92AXXTM9vELWKwnSzuOnR6/5cZ mc2zA+zN4FthnDovjCH7P1e2UvlXg38Pt0TymVSOtYIRSt04sw23J3ST5 VZITahjVKYqCzHhF9VV7RD9bRa/GRIdB8V8bTRCrSxhr/teJO/Wb3Z0l2 w==; X-IronPort-AV: E=McAfee;i="6600,9927,10855"; a="470176976" X-IronPort-AV: E=Sophos;i="6.03,205,1694761200"; d="scan'208";a="470176976" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Oct 2023 01:57:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10855"; a="746140872" X-IronPort-AV: E=Sophos;i="6.03,205,1694761200"; d="scan'208";a="746140872" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Oct 2023 01:57:23 -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 v5 1/2] mm/memory_hotplug: split memmap_on_memory requests across memblocks References: <20231005-vv-kmem_memmap-v5-0-a54d1981f0a3@intel.com> <20231005-vv-kmem_memmap-v5-1-a54d1981f0a3@intel.com> Date: Sat, 07 Oct 2023 16:55:19 +0800 In-Reply-To: <20231005-vv-kmem_memmap-v5-1-a54d1981f0a3@intel.com> (Vishal Verma's message of "Thu, 5 Oct 2023 12:31:39 -0600") Message-ID: <87jzrylslk.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: C50FE120003 X-Stat-Signature: wcfptaukdptso115oukyxjxfe8a8ocrn X-Rspam-User: X-HE-Tag: 1696669049-641822 X-HE-Meta: U2FsdGVkX18hZo81CFbS4aqQvCT3zaAmfkVowKVzVBtX5eWqw89JsrI/XcsKk1KG3y7R18UChj6TsJucSn7ESCRF1AEyV/DfACrogxXBMiu9yglGbQoszoRUv0jPdGO6m85Gjx4LuTAsNWSCgr9XhczRac8i0JTJsjw4mLu7fvsCJWz3W6H8vwYhuFLgNcY4WLE+/4wcf/TB4NkzeFx9YYU9IQKv4zMZPOspYT7AhDOB773eWl/g/xnjdmfrWs0yl6M9o/Cc4gtu2dPW3b1us83B9TPVeVAQlCFinUIOF/1w81SlKaoZYfV1TFAooXq3SG27J1k0NJM4Yd84PK4ksE3ZoRjKOz/4jUT4kIEPlIgiU8XnjtgNk1NSrQXwSiQjtNGCFmCHiwuFzMxkjf5+i68RLYxGTKDGlNwrRUvG65YWHLJA4lZJ2UUmMkcJtbFcKsWNgmtCW7pgr4P6J5VPUY2U4t7UOZzaq57h8VZNMbC5qjOGSp+DVfByz9I5JauddBbELom2qJ1Vqg45BR0yRmrAk4sxuYpRLW06ScCbj0Hci/nhiN20ru99+eh0tpZYW3Wupy7VPT+2tii8ksBrwHERq44cvFkzoqN0CZemDSYtbmdkcWTJETbrH92FkE9/As0Y1IRKbq2NqzYbHoOXTTjyIozcd2XA+XubdWeJFCIhoLFN7KSMUvNPBMyn3p5SFk5s1QeLcqudiOZ5LuNMEY8VN+82+ErV9HAAfHHSsptmyvWfgk5f/Q+X8jaqtzzAt5WYrRLIXIADL8X7CHefa3QUGtFV4steHcIO1YRRfevZZq83uNloszggw1vB4lvxpriJCJbi2PxoHMTu5H3BBEXS7xo1hXVmRWDMoH/btsNlS2Z0U6GJRgI7IelmdKNAGUaKmRjV6RB/vh8AUghJzY9Kv4bHEn/pPqDKCUH4gjw1HCP7uXBjedVA4HE9A4v/Mw6IBVIsCLD10+7k4HA 9e/EXfVH X87kIB+le0hqpWXUpvQ7ekQ3ouJID1gOZlrAyn+FhoRHQYI9EAC1M0e4NubyDE9VQuPlLL5A0RivQpp9q4bZUscKUQdl1g5kyaiAEx8RKySx3w5Li548tX4ea/KaZ3zwdNQ/lwkfD7nHubit4WOB0ssBCo4iW/D60ZXvZL09rAg6O+jZ3iMPqx74M/UUtFWyJrWw09Wp65eDVjWI/eUXudDvGQVADP//pPL6NeoU6TYOXzKjiPHZZLfSkI/WK3+C5jXeYlN6MOdloCnTSdtUEfKoITtIg5cqq9VBlCyxgddBEzU3odUz5Lof+Jwubf6zAAxfPN9gx6Gu9MfDUAN0y2VgVRVEn0WmD8a1nPqAbDFhWhqo= 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: 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. > > 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 > Signed-off-by: Vishal Verma > --- > mm/memory_hotplug.c | 162 ++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 99 insertions(+), 63 deletions(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index f8d3e7427e32..77ec6f15f943 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -1380,6 +1380,44 @@ static bool mhp_supports_memmap_on_memory(unsigned long size) > return arch_supports_memmap_on_memory(vmemmap_size); > } > > +static int add_memory_create_devices(int nid, struct memory_group *group, > + u64 start, u64 size, mhp_t mhp_flags) > +{ > + struct mhp_params params = { .pgprot = pgprot_mhp(PAGE_KERNEL) }; > + struct vmem_altmap mhp_altmap = { > + .base_pfn = PHYS_PFN(start), > + .end_pfn = PHYS_PFN(start + size - 1), > + }; > + int ret; > + > + if ((mhp_flags & MHP_MEMMAP_ON_MEMORY)) { > + mhp_altmap.free = memory_block_memmap_on_memory_pages(); > + params.altmap = kmalloc(sizeof(struct vmem_altmap), GFP_KERNEL); > + if (!params.altmap) > + return -ENOMEM; > + > + memcpy(params.altmap, &mhp_altmap, sizeof(mhp_altmap)); > + } > + > + /* call arch's memory hotadd */ > + 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, params.altmap, group); > + if (ret) > + goto err_bdev; > + > + return 0; > + > +err_bdev: > + arch_remove_memory(start, size, NULL); > +error: > + kfree(params.altmap); > + return ret; > +} > + > /* > * NOTE: The caller must call lock_device_hotplug() to serialize hotplug > * and online/offline operations (triggered e.g. by sysfs). > @@ -1388,14 +1426,10 @@ static bool mhp_supports_memmap_on_memory(unsigned long size) > */ > int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags) > { > - struct mhp_params params = { .pgprot = pgprot_mhp(PAGE_KERNEL) }; > + unsigned long memblock_size = memory_block_size_bytes(); > 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; > + u64 start, size, cur_start; > bool new_node = false; > int ret; > > @@ -1436,28 +1470,21 @@ 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 = kmalloc(sizeof(struct vmem_altmap), GFP_KERNEL); > - if (!params.altmap) > + if ((mhp_flags & MHP_MEMMAP_ON_MEMORY) && > + mhp_supports_memmap_on_memory(memblock_size)) { > + for (cur_start = start; cur_start < start + size; > + cur_start += memblock_size) { > + ret = add_memory_create_devices(nid, group, cur_start, > + memblock_size, > + mhp_flags); > + if (ret) > goto error; > - > - memcpy(params.altmap, &mhp_altmap, sizeof(mhp_altmap)); > } > - /* 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; > + } else { > + ret = add_memory_create_devices(nid, group, start, size, > + mhp_flags); > + if (ret) > + goto error; > } > > if (new_node) { > @@ -1494,8 +1521,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); > @@ -2146,12 +2171,41 @@ 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_block_and_altmap(int nid, u64 start, u64 size) > { > + int rc = 0; > struct memory_block *mem; > - int rc = 0, nid = NUMA_NO_NODE; > struct vmem_altmap *altmap = NULL; > > + rc = walk_memory_blocks(start, 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; > + } > + > + /* > + * Memory block device removal under the device_hotplug_lock is > + * a barrier against racing online attempts. > + */ > + remove_memory_block_devices(start, size); > + > + 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); > + } > +} > + > +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,47 +2221,28 @@ static int __ref try_remove_memory(u64 start, u64 size) > if (rc) > return rc; > > + mem_hotplug_begin(); > + > /* > - * We only support removing memory added with MHP_MEMMAP_ON_MEMORY in > - * the same granularity it was added - a single memory block. > + * 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. > */ > if (mhp_memmap_on_memory()) { IIUC, even if mhp_memmap_on_memory() returns true, it's still possible that the memmap is put in DRAM after [2/2]. So that, arch_remove_memory() are called for each memory block unnecessarily. Can we detect this (via altmap?) and call remove_memory_block_and_altmap() for the whole range? > - 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; > - } > + unsigned long memblock_size = memory_block_size_bytes(); > + u64 cur_start; > + > + for (cur_start = start; cur_start < start + size; > + cur_start += memblock_size) > + remove_memory_block_and_altmap(nid, cur_start, > + memblock_size); > + } else { > + remove_memory_block_and_altmap(nid, start, size); > } > > /* 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); > - } > - > if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK)) { > memblock_phys_free(start, size); > memblock_remove(start, size); > @@ -2219,6 +2254,7 @@ static int __ref try_remove_memory(u64 start, u64 size) > try_offline_node(nid); > > mem_hotplug_done(); > + > return 0; > } -- Best Regards, Huang, Ying