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 3F9CECDB484 for ; Thu, 12 Oct 2023 08:40:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B2C138D0119; Thu, 12 Oct 2023 04:40:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AB37F8D0002; Thu, 12 Oct 2023 04:40:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 97BC68D0119; Thu, 12 Oct 2023 04:40:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 88A8A8D0002 for ; Thu, 12 Oct 2023 04:40:58 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 5DDA580511 for ; Thu, 12 Oct 2023 08:40:58 +0000 (UTC) X-FDA: 81336164196.14.1202548 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf07.hostedemail.com (Postfix) with ESMTP id 1E5EB40012 for ; Thu, 12 Oct 2023 08:40:55 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=f87LYifP; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf07.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697100056; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=oUu9gspJCKmtseGxPT5e7VM//lOrd9x+RY45RlgrqZo=; b=wypwpSxKXQ+eplJo9MN46S2+Pi3R9dFvagpOxp0gff0kj47An10iFKBVY78WhE4Nsb+1Fw K+06niwEEbaHL/42EgqSq+M6zLthcxp29B+U/jUVuF++ZXhJaTUMPTOgTBjlspRBs3kELc EoXg8opJQk2GizYslc79soXnGK0gWMo= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=f87LYifP; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf07.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697100056; a=rsa-sha256; cv=none; b=BseFOkIV8ni1wYV7nwZYNmge1JScx0VVl6sZOpCzpxYP4WpLu468OtzsPws/FmHoaIo22h sNLdmT1IdIO0ejdk3J1So8sq7dvEvYyJvIEKxEmXU+F435Pf28SYXhH6Zut9bnad07GxCX G7V+NlEUY5+tnFymvU0a7sbIsA3vFvE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1697100055; h=from:from: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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oUu9gspJCKmtseGxPT5e7VM//lOrd9x+RY45RlgrqZo=; b=f87LYifP/I8G1HGoS34o1xbxH5IL+LIZ/IDI//za907i7yePj2T53OaVVJjhk3q4PtgTZr YJL5U6thpu0tZWX3KaogvHl6JNZd/YYYGVXYOzjd1dIC27hWFIvca4i3XB2b7fMxQaVWqD Iu6ibEqEtc3qH8WdhenVLDQcPiCtj44= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-629-DwtHduPJO2KIBe_X3e5FHg-1; Thu, 12 Oct 2023 04:40:53 -0400 X-MC-Unique: DwtHduPJO2KIBe_X3e5FHg-1 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-3247f646affso289680f8f.1 for ; Thu, 12 Oct 2023 01:40:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697100052; x=1697704852; h=content-transfer-encoding:in-reply-to:subject:organization:from :references:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=oUu9gspJCKmtseGxPT5e7VM//lOrd9x+RY45RlgrqZo=; b=VsonsCaoFHSJGw6EXzALZTi8JROHxXgYki4f6B3l45dCqYz0u1AcCnDcoQ6ibIe0DW G1KZrdtg03kL6yUyuJeW7951fwqrNgutFZoRiVRiyCyszkC1CVMF/VUET0Qyt8LuyvsW lRnkApSqSV7qw4egdyu3nA5D2u3h8WTWz7LBn+yECTb14QzF3Zbdhtkn2mTCpafPbWOx 9iVhFQxXCIARKMQ1pO6oBnZp4xgDiLXIbyrlgEgwLWmPLtstpSJ7grCkYqSS/PpvxswU KxN0LNKRh8U5IvTokJnjo+Tww+Nx+/pdDR7dSl2eNpgn16h7Dgq3q7vSPLDxa/SxScJo VOiw== X-Gm-Message-State: AOJu0Yxmg6y/q4xKXQpSEm+kZ8jxr29hBlFo0wYHXBz6kNtOcHA96eio QlEevtORw5l2BSRi3VH4B6ay+ELQEkG3Mj4L0b211jm1qxUTuFvLsdu4tkrg8ebHSl/Uwc3tbf0 G4pUBNomtoc4= X-Received: by 2002:adf:fd0d:0:b0:32d:92fc:a625 with SMTP id e13-20020adffd0d000000b0032d92fca625mr724714wrr.24.1697100052412; Thu, 12 Oct 2023 01:40:52 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHnsVLUc9SJqZHyOsUziQY+Ca/ePvrWEabXeptfmw7K7cVLyJaUKLY9iF+clfj3wVEPiBqd5w== X-Received: by 2002:adf:fd0d:0:b0:32d:92fc:a625 with SMTP id e13-20020adffd0d000000b0032d92fca625mr724682wrr.24.1697100051973; Thu, 12 Oct 2023 01:40:51 -0700 (PDT) Received: from ?IPV6:2003:cb:c70d:ee00:b271:fb6c:a931:4769? (p200300cbc70dee00b271fb6ca9314769.dip0.t-ipconnect.de. [2003:cb:c70d:ee00:b271:fb6c:a931:4769]) by smtp.gmail.com with ESMTPSA id g8-20020a5d5408000000b0031c5b380291sm17680891wrv.110.2023.10.12.01.40.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 12 Oct 2023 01:40:51 -0700 (PDT) Message-ID: <748d40cb-35f4-98d6-a940-055de88bbc8b@redhat.com> Date: Thu, 12 Oct 2023 10:40:50 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 To: "Verma, Vishal L" , "Huang, Ying" Cc: "Hocko, Michal" , "Jiang, Dave" , "linux-mm@kvack.org" , "osalvador@suse.de" , "linux-cxl@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "Williams, Dan J" , "akpm@linux-foundation.org" , "nvdimm@lists.linux.dev" , "aneesh.kumar@linux.ibm.com" , "dave.hansen@linux.intel.com" , "jmoyer@redhat.com" , "Jonathan.Cameron@Huawei.com" References: <20231005-vv-kmem_memmap-v5-0-a54d1981f0a3@intel.com> <20231005-vv-kmem_memmap-v5-1-a54d1981f0a3@intel.com> <87jzrylslk.fsf@yhuang6-desk2.ccr.corp.intel.com> <831b9b12-08fe-f5dc-f21d-83284b0aee8a@redhat.com> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v5 1/2] mm/memory_hotplug: split memmap_on_memory requests across memblocks In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 1E5EB40012 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: urg6373e99ybt1mhtibe67qdj1codfz7 X-HE-Tag: 1697100055-889692 X-HE-Meta: U2FsdGVkX1/9HmmhZtWdKJYV7lhjKxonjsrg+hMUccnZ45UWXBo6XQDAlElZKFI5At1xpNCW2NQ9oJ44aG9VD4ZT7Dq+aULFVhxkJkS77SRxCG6k4Ms6ruNmyTbMKjlDYcuG+6BhrOyoKR2A5Yzs9CjU6VB1UgeCYwB4Px/PN/DGfmBKq2VROtsKiB+hisTxTYRYDt3Syk7FAEqRv6PaLJAlmcsI/XheRgWPSJmCryxV3e5LbiZ+f2L6FdGjwstZHMl80QaDRhbpzdQ8aBXlX68AdhK1tc6YnKy6bPrjybReZEPrKqEs/sB7Jt34zv455eU07MAmUnlWXhA4itHpm6FU/bCrf4U5oPEqsSxf13I6GH8r5Ne0pWMdKqV9/ZAsMEyrnbZ0ohntZjrSaSDYGdwdwBYiIawBKEugUWyVP57Rs+cR+YGjYTcGUR6gP9qZ64m9RDGlLJJYP3MiQ3SpOwLg+CyviyhZo4fWLB5WX/dIH9oXIhKVFLy5ZiKZjOZHu9gitRQDMqgzvEh/y4Sbhu8PNJ76PgcWIZPB7BcqBIwSqABpiU86ytMATl8paSkfDDROOPvvSEwgmrnFAmb9HvUi2KcpvIOC17C/nuSC1seIidqLC1wCZofB7xi3HJaUW/uL2oIymdstT4+ZLnsd34CfdYreYDReZH2CaZl3aAgC5EtJCKYgBtggXhSs1lPVzI7f2sL3Q2xG/g7T/2lgm4eMQlnksUv/RJZCSI7PbAML1Re0SQEHUDarWO4ntFEaT+U20i4XZT6YqMAh3DRt2KiMdI7Mm6ztZN5rOK/KvEuey9U7xNseeoYxgttfYQzZVQRvH57TnJG0/rB/9yE5OmZkDAhvOaNlEMZm+F8M8yAExGmv6lhUrmmzJpvfrjg9jGD4ImIdhSjRE6w0jZebajO/4mi/JyTCW8sVxEz6e8SAGwItTe+4NOaSa1uZTbMxaLssUQDPCuu0jnGtGce YItdv0tR ed/pO7cI7SHbYu+CZ8Yg3lfN1gItgXSVPcB9IWq+hibshsh6sILTyEfPCkv5ui16IBjgAwt8NndP2JyFVkgoMWuI4J8ZO68gEm0wk6ZiIK4PgulS51eUi/0ijuTk+Ke293nQSaQKZn4Lt+KH4XKNkN3rEKcf8+YxSae63QuDDKRGsjchHKdLveYMy9YI7Fj3oY1FBv37me40DxhEvielOALRMp6O6vnKNo0wBS418JHX26L45+nOlJUmZlneBl0SdvhaKeGHVG+Cq1PXD8SCB9t4dxC1zM14CZ4ZlHgfVeeb0olJJrwnTFT86G4HcoIjIZWfGf2rX9zYDLAMhd4dNSPiLHZb/dMZQ05DxMWp2AwC6qvNRWTjwoAzf+zskUyjllGbTXthfR7Y25YlwzlWDj6ZkTCfwPLLIxpoKzT9BinfMf2pbNdkhTmtM6KW+pUbnr8E3lUHx9dJlFBOdOYzG27ovGw2xjLuqwEcnW/7zX2BN2HRrVMlWSsdjDw== 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: On 12.10.23 07:53, Verma, Vishal L wrote: > On Mon, 2023-10-09 at 17:04 +0200, David Hildenbrand wrote: >> On 07.10.23 10:55, Huang, Ying wrote: >>> Vishal Verma writes: >>> >>>> @@ -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? >> >> Good point. We should handle memblock-per-memblock onny if we have to >> handle the altmap. Otherwise, just call a separate function that doesn't >> care about -- e.g., called remove_memory_blocks_no_altmap(). >> >> We could simply walk all memory blocks and make sure either all have an >> altmap or none has an altmap. If there is a mix, we should bail out with >> WARN_ON_ONCE(). >> > Ok I think I follow - based on both of these threads, here's my > understanding in an incremental diff from the original patches (may not > apply directly as I've already committed changes from the other bits of > feedback - but this should provide an idea of the direction) - > > --- > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 507291e44c0b..30addcb063b4 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -2201,6 +2201,40 @@ static void __ref remove_memory_block_and_altmap(u64 start, u64 size) > } > } > > +static bool memblocks_have_altmaps(u64 start, u64 size) > +{ > + unsigned long memblock_size = memory_block_size_bytes(); > + u64 num_altmaps = 0, num_no_altmaps = 0; > + struct memory_block *mem; > + u64 cur_start; > + int rc = 0; > + > + if (!mhp_memmap_on_memory()) > + return false; Probably can remove that, checked by the caller. (or drop the one in the caller) > + > + for (cur_start = start; cur_start < start + size; > + cur_start += memblock_size) { > + if (walk_memory_blocks(cur_start, memblock_size, &mem, > + test_has_altmap_cb)) > + num_altmaps++; > + else > + num_no_altmaps++; > + } You should do that without the outer loop, by doing the counting in the callback function instead. > + > + if (!num_altmaps && num_no_altmaps > 0) > + return false; > + > + if (!num_no_altmaps && num_altmaps > 0) > + return true; > + > + /* > + * If there is a mix of memblocks with and without altmaps, > + * something has gone very wrong. WARN and bail. > + */ > + WARN_ONCE(1, "memblocks have a mix of missing and present altmaps"); It would be better if we could even make try_remove_memory() fail in this case. > + return false; > +} > + > static int __ref try_remove_memory(u64 start, u64 size) > { > int rc, nid = NUMA_NO_NODE; > @@ -2230,7 +2264,7 @@ static int __ref try_remove_memory(u64 start, u64 size) > * a per-memblock basis. Loop through the entire range if so, > * and remove each memblock and its altmap. > */ > - if (mhp_memmap_on_memory()) { > + if (mhp_memmap_on_memory() && memblocks_have_altmaps(start, size)) { > unsigned long memblock_size = memory_block_size_bytes(); > u64 cur_start; > > @@ -2239,7 +2273,8 @@ static int __ref try_remove_memory(u64 start, u64 size) > remove_memory_block_and_altmap(cur_start, > memblock_size); ^ probably cleaner move the loop into remove_memory_block_and_altmap() and call it remove_memory_blocks_and_altmaps(start, size) instead. > } else { > - remove_memory_block_and_altmap(start, size); > + remove_memory_block_devices(start, size); > + arch_remove_memory(start, size, NULL); > } > > if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK)) { > -- Cheers, David / dhildenb