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 C16E8C4332F for ; Fri, 3 Nov 2023 16:43:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 28B958D00C5; Fri, 3 Nov 2023 12:43:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 239DF8D000C; Fri, 3 Nov 2023 12:43:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0DBA98D00C5; Fri, 3 Nov 2023 12:43:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id F23FD8D000C for ; Fri, 3 Nov 2023 12:43:15 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id BA1B2B6528 for ; Fri, 3 Nov 2023 16:43:15 +0000 (UTC) X-FDA: 81417213150.16.95430CA Received: from mail-pl1-f179.google.com (mail-pl1-f179.google.com [209.85.214.179]) by imf29.hostedemail.com (Postfix) with ESMTP id B6BB712001F for ; Fri, 3 Nov 2023 16:43:12 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gydqqHPM; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf29.hostedemail.com: domain of nifan.cxl@gmail.com designates 209.85.214.179 as permitted sender) smtp.mailfrom=nifan.cxl@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1699029792; 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=ih7WlNmerHDqwU0NzEXfphRvrs+9wIcIe2yQovPxT6Y=; b=dA9n6MPhvS7vaNwTFbZCvOleIKP7wUcgDQ43rof0KNC5VdY7O2y3l3B5P78tzVuUbw+4wF HcQz+H1VGWumPDNR095342S2Osgldts7Engm+ZRzzHO1kAy/KDsOTu2qhHTV6unMmNZimK g6Oww+tT+b1rkccoeqHrx9z2yipc7No= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gydqqHPM; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf29.hostedemail.com: domain of nifan.cxl@gmail.com designates 209.85.214.179 as permitted sender) smtp.mailfrom=nifan.cxl@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1699029792; a=rsa-sha256; cv=none; b=IyWNL6cNLh00074DsFpeaIkARnxaZnPo8mkK1f+TSG7v6sb0JMo91PrlN5nc1dw+XcUgDP iP1tJtYEOScXdAbvnVFQdiUa6LuGd6w+OFA8GzsjF99xTmj3N/HBq9Cl+m8IwY71g7ubn7 JskKyv8NvcBdxNhiQF9lYfGGeO9frLg= Received: by mail-pl1-f179.google.com with SMTP id d9443c01a7336-1cc53d0030fso18814275ad.0 for ; Fri, 03 Nov 2023 09:43:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699029791; x=1699634591; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:date:from:from:to:cc:subject:date:message-id:reply-to; bh=ih7WlNmerHDqwU0NzEXfphRvrs+9wIcIe2yQovPxT6Y=; b=gydqqHPMwasUJbbIuvgPeGVEnHcMb4WJ1/+h9ja39dgnFtJKy2LzhQ+60LaoVnIvjI ap51o/+6ayi/t8KtMEFNG/T+5doCtMDs+3wF036oBgNLa31a23LUk3n6nIubJdHQ3lAW DKIheMJk2TKBDEOOncWV/ITjmstaQ5u3MgNeXrQXr5x5BZDv1NvIiF2avjkRn64dUDlC SnCmbLFM00DocSOLfwTVKix+9vAtcPx4xsGLTZFZJLV1uKySfrb5FxVSIWc4lC+MSG77 bVpk6h1bHHB8fXwm0D3A3EbZ9ntJMMj1Co2Pvp5aCDS/0w+d500W+lhzO06L9KSe7Anw SN1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699029791; x=1699634591; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:date:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ih7WlNmerHDqwU0NzEXfphRvrs+9wIcIe2yQovPxT6Y=; b=t3y5paXYQ44HYk7XEwBdyd7TAPFatcagQTokbkLphYzPeNEA+PrYRKJk1g0Rzl6hM/ /AdPTSDXBSaemISYMliXB2hQAh+SWLjf7fU0zmOUKe9+o5hTu0hanZf5cXqY8q0SyjQb OxiBFlm8guy6XZYj2mCtPL0nDIAE8vNQt3g5PSYN36pdZ400BdvUKN8StwyZ7xfYJbHG RWc6GOk0hpdkVx7S5bNYKioav8O2h60F1O3S6FH9neYQN5WG88zekTUa5nCmF+5r6UKe AWdkrZCS/k8SJ0/oRuGMtL1WgH7BhIfADZmYP7DkdPx+j2FRYfu1+W2ta1v+8fXYCy4e cOAQ== X-Gm-Message-State: AOJu0Yxz/q9UV00yCZZ1PtKEuWcoP6PIocsVgxlro92jbEXS1WPgU/pY vbUtj1OmFib1EIMSmI/r4OVmkTe2HwM= X-Google-Smtp-Source: AGHT+IHTY2DwZYDb52LHJUlwhpLWjT3LP+Yjv3rvTMU9Z3tdBf3bcsoesLUWFtnMH+nusSYEVmCOag== X-Received: by 2002:a17:902:e744:b0:1cc:431f:55e3 with SMTP id p4-20020a170902e74400b001cc431f55e3mr3164775plf.28.1699029791458; Fri, 03 Nov 2023 09:43:11 -0700 (PDT) Received: from debian (c-71-202-158-162.hsd1.ca.comcast.net. [71.202.158.162]) by smtp.gmail.com with ESMTPSA id w21-20020a1709029a9500b001bb9bc8d232sm1605796plp.61.2023.11.03.09.43.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Nov 2023 09:43:11 -0700 (PDT) From: fan X-Google-Original-From: fan Date: Fri, 3 Nov 2023 09:43:06 -0700 To: Vishal Verma Cc: Andrew Morton , David Hildenbrand , Oscar Salvador , Dan Williams , Dave Jiang , linux-kernel@vger.kernel.org, linux-mm@kvack.org, nvdimm@lists.linux.dev, linux-cxl@vger.kernel.org, Huang Ying , Dave Hansen , "Aneesh Kumar K.V" , Michal Hocko , Jonathan Cameron , Jeff Moyer Subject: Re: [PATCH v8 2/3] mm/memory_hotplug: split memmap_on_memory requests across memblocks Message-ID: References: <20231101-vv-kmem_memmap-v8-0-5e4a83331388@intel.com> <20231101-vv-kmem_memmap-v8-2-5e4a83331388@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231101-vv-kmem_memmap-v8-2-5e4a83331388@intel.com> X-Rspamd-Queue-Id: B6BB712001F X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 7cxp85e9pb6faxxyo3pxyy5h7ck394wm X-HE-Tag: 1699029792-985503 X-HE-Meta: U2FsdGVkX1+iflvdWUWIHUYITTVVS+g2EuakZX3CVem5ThUkA96/TlPHsDiLFnOvawF8UVcIs+hJIMm9f/WaVlFCVLciiaFzdeFwsANIbbilgD38UPmah6Pt0IguQcJU6IR3gqOF8yBXo054MWcgyz/QAEspkDR07nP3eoESF8JXt+/0EloiW2YENmNil47m8ctYCOotpr7aDv/clpx/USRuVnAH74hrNGoxkrLPEdmTGy+VLK6/duIc9qFCf3kdrnEzmJVIe1ds0CAgFlViX9FrYG2N31M9KLbnwhk/q1cB86rsi7998CLQ2kWoo1lJ2xLogGVKnp5Pp3yiRuECm705B7CzDjBG/COX5Pf/Dr21tmKu8tOzdVS++8AAglRClwDgDSArPq/tvFxQqX/uklPHcTMd/toInmHlgLCFarOIYZDo2By1N5zOt/IlX8G95b91Dsns9C6S+hHFgrDR6pUNtg7b5kK62i9Zj//hdQD/iG7wWfoTSjhwerlZ/jCPKp994jIxtQac2t+FxwBd5qH2H7UzWP82SzzpYd6kXf9U2qLhsyZG3RNrFeo9Xz0g7i1pjhcN558fPFtgZYsuv+2CCuZm/8ZS6MZVuqeZPbd2ewZMUEfl5mCVI7bMQAVYaIiQKG7qpVpt8yBhJ+oT479p2vr7W30yvvEw5lozfzYDwhQzlJsVGSVLieqBZrLGlrEv9MTQOhAk+COCS48GBRgZ1Rm0Q+EifOCEQVvDhmbyf+HbeTtjQqvdj/VI9sOZVMfwQUxbaz2fSdB1S3cDMU5aW1RdDJ2JzFIRv0WpKbf/Zy1VZrmmE1lBFYalzyxrh3dDzVISWB5lZrVNmdS60OPGdqIvMQgbrrdb0AhFdg5V56HYbs6KvwJ8erZzspmDhBeyudFzLH+wBILFsxbcMkXeXQVdghW9YIw5/ofJECJHBH8y9NxcATko3oevPshoVgpj1ZQhSQh+/qxVsKz hrZ1RiZz GtjIpA25wYTrihnOn26qDZUSsfbI0YXD+MBopWCVULsuoy2Td+DSf+EJsPJ+7HRQh1m3IIuEWeA2UKZpGT6GRcBxUn0sKiSI0DnfgyWZ0Zaqj6PMBLYrQ3UBiIXmfdvna7RX4aRCBJRauXjVAR0Ru6e4ZUQaDAe3feF+gdAjFQli7cDhlg/YA8AY0M1b+fhf6+C+6g7UdRMnmKtmqmBRv3oKldyk0wtl8Cw3cV8Q0HdupgEVKfCzB5T/pWjncGuHpV+XHmLPVRpE3v47r5pkoWeMujovlKLylo/FHYrhFEk6euEzmsV/aK/i/7DDdyHd5alymrWHLeiWX8L83k2HfvY2zz/sprIL6IAcDMiiFEVesTBLDhRk/nue1e7K95wGH0D3KfH5LQmSNSX17ZnmDM/7HZSuOB9kD8Vn7chRWw6GsBaorA3xg/SbuutTstu37dxekxKwko/bj3E+2PgUZYaNZzqVV/qUrhWbeV5ef4uPEc0vE5zgn8K/c0tRN+Vglc/RitRK/w4qWxYLxR3GENG7PvZG5L0NeCOCPiu+QyVXh5XGjFXH01zTHNbFC4DijLnOw1c7j8u7HFhdhoplNfXQU7BDkos/b473CZEXx1/eZaSR4MxHBr3YExktXrTARl8eS6rE9q7drDxa9tV0qjOBrod73y0ocqKwDdIkDtRgA4kVXf2A3XHQrkuMR1LKYzcnqRWfK/makZeqWuAHuFT333YzfqJShwH6x+SwMrlhe3Hg= 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: On Wed, Nov 01, 2023 at 04:51:52PM -0600, Vishal Verma wrote: > 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 | 213 ++++++++++++++++++++++++++++++++++------------------ > 1 file changed, 138 insertions(+), 75 deletions(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 6be7de9efa55..d242e49d7f7b 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -1380,6 +1380,84 @@ static bool mhp_supports_memmap_on_memory(unsigned long size) > return arch_supports_memmap_on_memory(vmemmap_size); > } > > +static void __ref remove_memory_blocks_and_altmaps(u64 start, u64 size) > +{ > + unsigned long memblock_size = memory_block_size_bytes(); > + u64 cur_start; > + > + /* > + * For memmap_on_memory, the altmaps were added on a per-memblock > + * basis; we have to process each individual memory block. > + */ > + for (cur_start = start; cur_start < start + size; > + cur_start += memblock_size) { > + struct vmem_altmap *altmap = NULL; > + struct memory_block *mem; > + > + mem = find_memory_block(pfn_to_section_nr(PFN_DOWN(cur_start))); > + WARN_ON_ONCE(!mem); > + if (!mem) > + continue; > + > + altmap = mem->altmap; > + mem->altmap = NULL; > + > + remove_memory_block_devices(cur_start, memblock_size); Is cur_start always aligned to memory_block_size_bytes? If not, the above function will return directly, is that a issue? Fan > + > + arch_remove_memory(cur_start, memblock_size, altmap); > + > + /* Verify that all vmemmap pages have actually been freed. */ > + WARN(altmap->alloc, "Altmap not fully unmapped"); > + kfree(altmap); > + } > +} > + > +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); > + goto out; > + } > + > + /* 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); > + goto out; > + } > + } > + > + return 0; > +out: > + if (ret && (cur_start != start)) > + remove_memory_blocks_and_altmaps(start, cur_start - start); > + return ret; > +} > + > /* > * NOTE: The caller must call lock_device_hotplug() to serialize hotplug > * and online/offline operations (triggered e.g. by sysfs). > @@ -1390,10 +1468,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 +1510,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 +1562,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); > @@ -2062,17 +2128,13 @@ static int check_memblock_offlined_cb(struct memory_block *mem, void *arg) > return 0; > } > > -static int test_has_altmap_cb(struct memory_block *mem, void *arg) > +static int count_memory_range_altmaps_cb(struct memory_block *mem, void *arg) > { > - struct memory_block **mem_ptr = (struct memory_block **)arg; > - /* > - * return the memblock if we have altmap > - * and break callback. > - */ > - if (mem->altmap) { > - *mem_ptr = mem; > - return 1; > - } > + u64 *num_altmaps = (u64 *)arg; > + > + if (mem->altmap) > + *num_altmaps += 1; > + > return 0; > } > > @@ -2146,11 +2208,31 @@ void try_offline_node(int nid) > } > EXPORT_SYMBOL(try_offline_node); > > +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) > { > - struct memory_block *mem; > - int rc = 0, nid = NUMA_NO_NODE; > - struct vmem_altmap *altmap = NULL; > + int rc, nid = NUMA_NO_NODE; > > BUG_ON(check_hotplug_memory_range(start, size)); > > @@ -2167,45 +2249,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 +2280,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); > } > > /** > > -- > 2.41.0 >