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 D3CACE81DE9 for ; Fri, 6 Oct 2023 12:52:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5AAFC8E001C; Fri, 6 Oct 2023 08:52:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 55AC18E000B; Fri, 6 Oct 2023 08:52:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3FB888E001C; Fri, 6 Oct 2023 08:52:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 2EE358E000B for ; Fri, 6 Oct 2023 08:52:27 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id DC9631A071D for ; Fri, 6 Oct 2023 12:52:26 +0000 (UTC) X-FDA: 81315025092.09.4961EB8 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf01.hostedemail.com (Postfix) with ESMTP id 9F53440010 for ; Fri, 6 Oct 2023 12:52:24 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=WBGP3LAZ; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf01.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=1696596744; a=rsa-sha256; cv=none; b=BR/rtGFibV2FqnQNI/Uw4Ap6/tnyQnBzmtzgHpFIkCKkZ6NyBdet7AHk7yslQlifiEROhg oLQnlhchSBHt4qhidSohGA/0UUXoRwFlmmNIiBwKXVFRtcopMf+3p4s0J4zeEkfDa+ox1+ n743THiMZo3M6aQQWDQCLuxafPcF4h0= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=WBGP3LAZ; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf01.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=1696596744; 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=8n0TI4SHYWR8D0+rFw5ZDTzVGQmlQiFrCFjY/Y1ZyN8=; b=uHBC3j9kYHWx+gbOxXAfPhPTKQMwHbhycz6u9S1tRRbCzDmLoLcu9U3M4C+LW/PpqMSVAS ta7yRgF3L4U8lkgYISSw1PSYTqjCgcdo+h+AeofQfdnuj6cyj9nZOI2EP9eBgD56EkQ8Ax lhyE4uLzP5n3itdIshRF7PzP711KdeQ= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1696596744; 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=8n0TI4SHYWR8D0+rFw5ZDTzVGQmlQiFrCFjY/Y1ZyN8=; b=WBGP3LAZy9oQg4pt2COoKvI/p3OHUrG3g4MU7BuJ7s/cbggfOiGKpnED4pvZD3h0gb7Thh 91OYM+UtnsaPPZn/HJLlffaAKur6iLiicXWgJG1VtW48NNKnadf7D1dac8OU5vUAMBs1uC Rx4jx+lxIiN6pZ+WS9TFYMgD7oI7vQM= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-103-6VOuzi8sNWejObYRRjhW9w-1; Fri, 06 Oct 2023 08:52:17 -0400 X-MC-Unique: 6VOuzi8sNWejObYRRjhW9w-1 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-4053a5c6a59so13689515e9.3 for ; Fri, 06 Oct 2023 05:52:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696596736; x=1697201536; 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=8n0TI4SHYWR8D0+rFw5ZDTzVGQmlQiFrCFjY/Y1ZyN8=; b=ijAPY9ACeny50FpfjXPBze05gctdFIKiPkQFSN/6FeusZpiXDU6XC9LN5LlIzYxLGa IFvt4cVH6CPPvO2WJ4HELrQj1yiZRyutjWnaTQdMOP2XIwHN11Tf2hO2E/uxu69om9IE 2DqFDuiAVeXyO4LNFKMZfxZ6OCnPEVgFqhF7PIS6g0gNlqwpYeZiKYzFH7aD0FUIxRCK QgfBqe5eszIWC7VXrsmyLUkQCyKo85p1iY6ccKmZ6kym+D+jUI2Wh2EJ3q/WtmIGZwlB 9idU/Hgo2CCJMTqxRHqcGhnzmHhPfcfakFNt/LbTf5eWEnPpRGsJ3WmTUKEi0YrVHcGj j5ZQ== X-Gm-Message-State: AOJu0YyFrAWJYKJGp//OQGw9zeDC5Oh46d7djKT4asHhpCStGaSgEVQr gLD0fnG473nGBkSqsqXQo0JpWWlgeb3fL1f6hZFm1i6wOTLdZOMajB9Bc8m6OoMM5L0hjhtG7uh 82Qr8Y47A9ko= X-Received: by 2002:adf:fd46:0:b0:31c:7001:3873 with SMTP id h6-20020adffd46000000b0031c70013873mr7396133wrs.60.1696596736389; Fri, 06 Oct 2023 05:52:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHMUjFqwG0bWRjll9bNskCayaIMukTFBBWv6LIwzEtvDAwr4vsLCz67BNLL4G0+qfj7f4GDKw== X-Received: by 2002:adf:fd46:0:b0:31c:7001:3873 with SMTP id h6-20020adffd46000000b0031c70013873mr7396109wrs.60.1696596735883; Fri, 06 Oct 2023 05:52:15 -0700 (PDT) Received: from ?IPV6:2003:cb:c715:ee00:4e24:cf8e:3de0:8819? (p200300cbc715ee004e24cf8e3de08819.dip0.t-ipconnect.de. [2003:cb:c715:ee00:4e24:cf8e:3de0:8819]) by smtp.gmail.com with ESMTPSA id a9-20020adfe5c9000000b003142e438e8csm1601359wrn.26.2023.10.06.05.52.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 06 Oct 2023 05:52:15 -0700 (PDT) Message-ID: <4ad40b9b-086b-e31f-34bd-c96550bb73e9@redhat.com> Date: Fri, 6 Oct 2023 14:52:14 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 To: Vishal Verma , Andrew Morton , Oscar Salvador , Dan Williams , Dave Jiang Cc: 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 References: <20231005-vv-kmem_memmap-v5-0-a54d1981f0a3@intel.com> <20231005-vv-kmem_memmap-v5-1-a54d1981f0a3@intel.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: <20231005-vv-kmem_memmap-v5-1-a54d1981f0a3@intel.com> 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: 7bit X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 9F53440010 X-Stat-Signature: ido6zofqcndnobh758ihraqbpa3n8zi4 X-HE-Tag: 1696596744-566442 X-HE-Meta: U2FsdGVkX18bxVC8iFEA5u0bJJEVPCQItMZPXKsY+W23I9vNGMsTsYesms7bdByi4J0wG1d1nqGE2ZmCVYfRRU9QtyISKktYCkm2v5MBGcgGiXVHNIfbf2ULESmmcxaTEal2K063InUPx4nO/tf1MitwIsyovkusrzUGxPorMcMS8jU5k1ghX9qLtjR/MuaFElzZObGpMtpu99OKcYuHr6GIoWv4h1GXxPdJ4KJKxI6JaTxV8O/hHIuVfD4RSEqW00loIAr6Try0AnvV1wvzriJUS0TR1fUyfDE/YIxyTpDJMiV2Ioq3QH665IcFeyutdJqFJuZBs4sdpUSw6xj5fNACDcZRyhHy7BAi4mj8y3sHmgCQ26qAlc72Od0vn/xMYFaig5dO3QpnZm66GGNns/ypbU8/Mgm2+ge08cOGX7BLNzmQJ7fgEJnkl48SPyEKJWse+KQLjqaIxk7jc1Bs02zTl5IaR4smdj4x4IO5ApAEKFcxBRenM8qDSkqTBqN+CYpFsBBQnLT/6iRqqpB05QqXLulTuEQ+e4VXISwm17yPtksERUGdtDhHjxZVgys6MDUUrGyRwXB4w1ARO9QoqG6ILHAq2LOZPF8h8rd2MoTlgh/LhVF2qrc2tO74q3s385JphUHrxJXg4t8hx9PVKLWssvz9fuv1YfBsYYZEpbS9A06Hf/LX0uD5G/isA1RYfKbIsoPFTGP+5fw06ewz19SFtiTf5k9EoLkCuOg5xxWIMYGtr6ExUCwWvMHJ2s42KsrGqdOghMRPGmlLiHiofzh1BZJUpFrbvVi/e2EnnYp4rV495kizttVuZynx0FcgoMJLkiCVscp1XebzXDv/YMcDo4S913/f+n0K6YZYBfM0F/gayKdXisPXWKz6aIOVoJggF5TA+7Pf8roCCXREeHcKTQg4VobskxHwPKBCqaWuoMgWJX5spRriQBDxrYDRE7xs+UsFV3v0OQOZx5E vA93vanm XaUjy+eXtAXw3sLallnq5zDARlRKLonVfBcbIa6jY5eNBqyGdkU32bptyBbxSypC1YlsqMSXpPCMhjduzq830iWAWeVi0lasDXibCVVjOacqiD7p3h2OS92UmBBy7fe6l3PhjLPqi2O1P16NdYV8qQZYhRwavZ4m6APqjM1xRXy2TIJll+JwZe6tt/hXyCwAZowtmKqg6JvvFBq9BbpGPY6CPx1oMhZWtJ9nBphi3qmNxIrVN3IE9O5rycvBl0cMOvj3vGFZPMpRI+dqlzwyT42pg3ev1vp9IcFg8Wzvxqd7LvQ/24Mr/poDsLoJVtUnEEhgOw3uYX2tD9CNDK9ttQBt7J+fr/Vg+eJqOKlqppXryO6cwg0ps5GLPCEnBGsK+WWprg0s7phvcDntqnRE105+NAWb6wyUeG1ozSrhkSh6Lw/9hh5o0yNooeA== 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 05.10.23 20:31, 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. > Maybe add that this implies that we're not making use of PUD mappings in the direct map yet, and link to the proposal on how we could optimize that eventually in the future. [...] > > -static int __ref try_remove_memory(u64 start, u64 size) > +static void __ref remove_memory_block_and_altmap(int nid, u64 start, u64 size) You shouldn't need the nid, right? > { > + 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); We're now calling that under the memory hotplug lock. I assume this is fine, but I remember some ugly lockdep details ...should be alright I guess. > + > + 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()) { > - 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); Better call remove_memory_block_devices() and arch_remove_memory(start, size, altmap) here explicitly instead of using remove_memory_block_and_altmap() that really can only handle a single memory block with any inputs. > } > > /* remove memmap entry */ > firmware_map_remove(start, start + size, "System RAM"); Can we continue doing that in the old order? (IOW before taking the lock?). > > - /* > - * 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(); > + Unrelated change. > return 0; > } > > -- Cheers, David / dhildenb