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 1BB37EB64DD for ; Thu, 13 Jul 2023 07:23:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 783456B0071; Thu, 13 Jul 2023 03:23:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 70BEF6B0072; Thu, 13 Jul 2023 03:23:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5ACA66B0074; Thu, 13 Jul 2023 03:23:31 -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 48F596B0071 for ; Thu, 13 Jul 2023 03:23:31 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 0D8BAA014D for ; Thu, 13 Jul 2023 07:23:31 +0000 (UTC) X-FDA: 81005748222.24.329E224 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf22.hostedemail.com (Postfix) with ESMTP id 5B038C000D for ; Thu, 13 Jul 2023 07:23:28 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=aoZqzl3+; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf22.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=1689233008; 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=DgHHNUMQekCLPFyLoFSxMBSAfdXYCTsaZ05jN1ABgoY=; b=T6P0PKXSxPcm0IsQMVT2q0Ki0mKB2Eh9A+9dHy1ly/7UhY3u/7a2En+CyDhnJIa5PPYiRe 7bO0EQ/jAzqFYh3FjQ/+LZ5Ptz1cwknDIRg46EPcB6lnIByQfEfLIghxrNnFuQNywlM4CR 6WjyRwpNIggKB00rkkO3wKRdziGBJFY= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=aoZqzl3+; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf22.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=1689233008; a=rsa-sha256; cv=none; b=FxbxRqwHnzyuTJWWy4NG1xG0rdSU93UmIxE7YAbMGk3DU7p0Qk/cnR4OVmO6Ml1EfxDpFX oxKrCqJZWg8PjZTE0KhSIxHkSkSg+kSqbjsDon4jyrBmFIhchDkUXmcELn3QFFcsIs9CYS upPOVodCL4tkYknboejIHFewr3IwgS4= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1689233007; 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=DgHHNUMQekCLPFyLoFSxMBSAfdXYCTsaZ05jN1ABgoY=; b=aoZqzl3+ZAwf6X+7CwmSRnD0nqaHNS0pXHY5LP/SNgISdpAYXTL/fr6Tp0nQkViAYZo1AI qSV8P7/vd+rn3cWNOH76syWLz7JddRCo75buZQdb0J4IeqBDnEYqvK/dkuO3l9zb+vg1Wl He82lwGbgZDKRhi8FHw9jXfEnEdRPLQ= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-185-88aTrN04Nx67YZ6gfjmtDw-1; Thu, 13 Jul 2023 03:23:25 -0400 X-MC-Unique: 88aTrN04Nx67YZ6gfjmtDw-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-3fc07d4c2f4so1715465e9.1 for ; Thu, 13 Jul 2023 00:23:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689233005; x=1691825005; h=content-transfer-encoding:in-reply-to:subject:organization:from :content-language:references:cc:to:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=DgHHNUMQekCLPFyLoFSxMBSAfdXYCTsaZ05jN1ABgoY=; b=ONLFDdW3xNQ8i2jSYyV/mT+nheI6A/J/DShRY65H6LABkNkWbE/n2Qi753Ai7HPKZl D5T2KMbb8zCaVC7cZOwNAG9zpbR+zMzN9fJAv8UBCjBtpo5SgXackYx/lzZ6OEA3tyVB 6nPoeY5iaqC4lfAy13ihLwMFdScuRjxjs55varNFpWi99zxZDTf7L6myAeulMI4GTDTp X0zWs1myazhoaoq64ldfWiVSrHoBUHhEZDEw9Wk+PyRoFSQs3Wk7nYQlj8BHnWPcabfD ZT6YtJFJlABMRS8He1fjpzfboUHE10/p8R8xTsMzSJR2mxDOQ348ZdqWvleqc3Jkr7Q5 ATVg== X-Gm-Message-State: ABy/qLaY9X9fbvM93+x6tHAaZtXxk1fyOU2sFKe6pJduaqu9nCbaIUNz vTGNcADO2j0VT2FO0YO3Vgjah2l8OAp9Q5HKzxBvxoyDhbZvLaT8A4NuKSUcJcUtGRz/VjLVUb7 e8zsOmfPiUSE= X-Received: by 2002:a7b:c8cf:0:b0:3fa:984d:7e99 with SMTP id f15-20020a7bc8cf000000b003fa984d7e99mr700227wml.22.1689233004717; Thu, 13 Jul 2023 00:23:24 -0700 (PDT) X-Google-Smtp-Source: APBJJlEHp1Dof1hZ5eLgX91wy7JzHUlL7TjMzwPfLhPZe9l5XN/YJjOHJ27klX1+mumUVy1mVfYk1Q== X-Received: by 2002:a7b:c8cf:0:b0:3fa:984d:7e99 with SMTP id f15-20020a7bc8cf000000b003fa984d7e99mr700199wml.22.1689233004294; Thu, 13 Jul 2023 00:23:24 -0700 (PDT) Received: from ?IPV6:2003:cb:c717:6100:2da7:427e:49a5:e07? (p200300cbc71761002da7427e49a50e07.dip0.t-ipconnect.de. [2003:cb:c717:6100:2da7:427e:49a5:e07]) by smtp.gmail.com with ESMTPSA id l15-20020a1c790f000000b003fc01f7b415sm15246910wme.39.2023.07.13.00.23.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 Jul 2023 00:23:23 -0700 (PDT) Message-ID: Date: Thu, 13 Jul 2023 09:23:22 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 To: "Verma, Vishal L" , "akpm@linux-foundation.org" , "rafael@kernel.org" , "osalvador@suse.de" , "aneesh.kumar@linux.ibm.com" , "Williams, Dan J" , "lenb@kernel.org" , "Jiang, Dave" Cc: "Huang, Ying" , "linux-mm@kvack.org" , "linux-cxl@vger.kernel.org" , "nvdimm@lists.linux.dev" , "linux-kernel@vger.kernel.org" , "linux-acpi@vger.kernel.org" , "dave.hansen@linux.intel.com" References: <20230613-vv-kmem_memmap-v1-0-f6de9c6af2c6@intel.com> <20230613-vv-kmem_memmap-v1-3-f6de9c6af2c6@intel.com> <87edleplkn.fsf@linux.ibm.com> <1df12885-9ae4-6aef-1a31-91ecd5a18d24@redhat.com> <5a8e9b1b6c8d6d9e5405ca35abb9be3ed09761c3.camel@intel.com> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH 3/3] dax/kmem: Always enroll hotplugged memory for memmap_on_memory In-Reply-To: <5a8e9b1b6c8d6d9e5405ca35abb9be3ed09761c3.camel@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: 8bit X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 5B038C000D X-Stat-Signature: i83feiays5x357zj4wd9eexspmndpdq8 X-HE-Tag: 1689233008-687124 X-HE-Meta: U2FsdGVkX18P/kWjO61TTRrCwmjw868p11X1oDR+ngkv41S5v8PyLioI1bDmSCBG+Mcuc1sXFPeqYXNNSQIJ6frAIxrx3XdlA/ysW/o3tTQy3ynzG0Bjnz/y1Nz7Mrub0jK5S7ni4q1WhF8TqXfmi07YcmgRuLhy2zIowdHN64KlaZq3NlH/4jvERUydCKkilyR+zWk+zPKTsDwiPWl8bMqt9UBXtdP0m6W/gWgswNUmxzd+gIETVFZ1sb+5RROi58oL/9CGMnTC8tO2aZfqIZnMFXS7Bt6+Y4c/wQekT92Vn4SXFrg4Z2sRISwKmpxhodUVNLQznqa2rriw6p81VJIR37gt7r7LdvFrzwYDxu4ysRAqxttpKPifIrIGtFL3vcHv69sKDp81JhU98WwaqAbt26EB0fFoRtYmmic0tYwrshAdUpaiL3Mz7UhJAVimcZiWqAp2+cOQglvoEutDXtlgAJIXBWEV+Z5PnM4UG3S6O3nPT9YsVis3htVqR4erTdwXif5++494dxEv/JZMhr8U4mCHH0B0iQoHsL0nIrkOBH7w+ccU/UMjsYfauT6Ldrz7eo3Xwv9rkUNOaHNhEi5go5WT+qFe3av3OCSCAoChGQwuQ4RGc2vOXosd3bay+EHBwgn6VUcsBVFKmSDIkf8SKXRLYR0p3/TIjmY5lZmEOjy/HUiJKzC0ql97Zri9UP5vjXq2zJW43qDkZPhXjIdJ5+ALtO3BQUaVfoyfQIVE5LrPuR2ydKWpHhS7Gp22RvP5n63OmtPUbB0VoDujVayms/IKGpyu5WuAUl/Q6YY1oDZ8exFxGHCqUf/lPj2vTJwtFnYb5skRlSB0b1ZbA7GA1z8/FTrP6ImU/4voi2PcmRgFT9Ma6CyVIyxlbtu509c+fhJMdRrbBiBXA/GdQTzuYQsTQmTRpgprN95mmFhWg6SlbGPJ2QlOQsNiUjtuoADoUfI2BBsKWTFVvsI 65zZ3zP5 3HosI7W3q4ptNXOlAUp6+WfHH+gJumZ31Gx2nOnmT9RdBIruCPoMrxgN5HeKSdaLHcmzgd4+YtFwks7+6QsYC8CSA2WYJE/QzuhqPWSU+cWeIBGzdLBpUlE4a2zPHqKOuF7Xsrd/BbA85zcG61YAxlHBEGtSA/3M6pZLPuzmOnt1HEzHN24tlkI+Ai2/5UGMRRxrnKuA2QRcm9LAzBCG0ZEYW5hTXILQnrdzpHE0aLbOJfgEPFxBzXCublpqkHIdi60X5st72pV1P0pvIhskhfPvwdFtHQsNi4OUUftohL5TmSq7bV2fRnqSAfFDxAb3lFPeFZWlXG5NNm9Txooxp8QiI1SY7Aksxe/RksMcm3vvexUNDOdgp8ef2ln8hCviXHYO/8chfhUVY41s3YX1jIUijLSOhhJiiChT5m2jllf+nkZ+ucg2WHf3edTmidcIC4jhqKxwWhpMhs7QvsMbms8Z6s3MJTxDmXpN7/z+YXqtktRUBEJKnPjA5mw== 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 13.07.23 08:45, Verma, Vishal L wrote: > On Tue, 2023-07-11 at 17:21 +0200, David Hildenbrand wrote: >> On 11.07.23 16:30, Aneesh Kumar K.V wrote: >>> David Hildenbrand writes: >>>> >>>> Maybe the better alternative is teach >>>> add_memory_resource()/try_remove_memory() to do that internally. >>>> >>>> In the add_memory_resource() case, it might be a loop around that >>>> memmap_on_memory + arch_add_memory code path (well, and the error path >>>> also needs adjustment): >>>> >>>>         /* >>>>          * Self hosted memmap array >>>>          */ >>>>         if (mhp_flags & MHP_MEMMAP_ON_MEMORY) { >>>>                 if (!mhp_supports_memmap_on_memory(size)) { >>>>                         ret = -EINVAL; >>>>                         goto error; >>>>                 } >>>>                 mhp_altmap.free = PHYS_PFN(size); >>>>                 mhp_altmap.base_pfn = PHYS_PFN(start); >>>>                 params.altmap = &mhp_altmap; >>>>         } >>>> >>>>         /* call arch's memory hotadd */ >>>>         ret = arch_add_memory(nid, start, size, ¶ms); >>>>         if (ret < 0) >>>>                 goto error; >>>> >>>> >>>> Note that we want to handle that on a per memory-block basis, because we >>>> don't want the vmemmap of memory block #2 to end up on memory block #1. >>>> It all gets messy with memory onlining/offlining etc otherwise ... >>>> >>> >>> I tried to implement this inside add_memory_driver_managed() and also >>> within dax/kmem. IMHO doing the error handling inside dax/kmem is >>> better. Here is how it looks: >>> >>> 1. If any blocks got added before (mapped > 0) we loop through all successful request_mem_regions >>> 2. For each succesful request_mem_regions if any blocks got added, we >>> keep the resource. If none got added, we will kfree the resource >>> >> >> Doing this unconditional splitting outside of >> add_memory_driver_managed() is undesirable for at least two reasons >> >> 1) You end up always creating individual entries in the resource tree >>     (/proc/iomem) even if MHP_MEMMAP_ON_MEMORY is not effective. >> 2) As we call arch_add_memory() in memory block granularity (e.g., 128 >>     MiB on x86), we might not make use of large PUDs (e.g., 1 GiB) in the >>     identify mapping -- even if MHP_MEMMAP_ON_MEMORY is not effective. >> >> While you could sense for support and do the split based on that, it >> will be beneficial for other users (especially DIMMs) if we do that >> internally -- where we already know if MHP_MEMMAP_ON_MEMORY can be >> effective or not. > > I'm taking a shot at implementing the splitting internally in > memory_hotplug.c. The caller (kmem) side does become trivial with this > approach, but there's a slight complication if I don't have the module > param override (patch 1 of this series). > > The kmem diff now looks like: > > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c > index 898ca9505754..8be932f63f90 100644 > --- a/drivers/dax/kmem.c > +++ b/drivers/dax/kmem.c > @@ -105,6 +105,8 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) > data->mgid = rc; > > for (i = 0; i < dev_dax->nr_range; i++) { > + mhp_t mhp_flags = MHP_NID_IS_MGID | MHP_MEMMAP_ON_MEMORY | > + MHP_SPLIT_MEMBLOCKS; > struct resource *res; > struct range range; > > @@ -141,7 +143,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) > * this as RAM automatically. > */ > rc = add_memory_driver_managed(data->mgid, range.start, > - range_len(&range), kmem_name, MHP_NID_IS_MGID); > + range_len(&range), kmem_name, mhp_flags); > > if (rc) { > dev_warn(dev, "mapping%d: %#llx-%#llx memory add failed\n", > > Why do we need the MHP_SPLIT_MEMBLOCKS? In add_memory_driver_managed(), if memmap_on_memory = 1 AND is effective for a single memory block, you can simply split up internally, no? Essentially in add_memory_resource() something like if (mhp_flags & MHP_MEMMAP_ON_MEMORY && mhp_supports_memmap_on_memory(memory_block_size_bytes())) { for (cur_start = start, cur_start < start + size; cur_start += memory_block_size_bytes()) { mhp_altmap.free = PHYS_PFN(memory_block_size_bytes()); mhp_altmap.base_pfn = PHYS_PFN(start); params.altmap = &mhp_altmap; ret = arch_add_memory(nid, start, memory_block_size_bytes(), ¶ms); if (ret < 0) ... ret = create_memory_block_devices(start, memory_block_size_bytes(), mhp_altmap.alloc, group); if (ret) ... } } else { /* old boring stuff */ } Of course, doing it a bit cleaner, factoring out adding of mem+creating devices into a helper so we can use it on the other path, avoiding repeating memory_block_size_bytes() ... If any adding of memory failed, we remove what we already added. That works, because as long as we're holding the relevant locks, memory cannot get onlined in the meantime. Then we only have to teach remove_memory() to deal with individual blocks when finding blocks that have an altmap. -- Cheers, David / dhildenb