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 A1FCEC636D6 for ; Thu, 9 Feb 2023 16:44:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EE4EE6B0071; Thu, 9 Feb 2023 11:44:26 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E951E6B0072; Thu, 9 Feb 2023 11:44:26 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D36C26B0074; Thu, 9 Feb 2023 11:44:26 -0500 (EST) 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 C5EB06B0071 for ; Thu, 9 Feb 2023 11:44:26 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 79EFB80D1C for ; Thu, 9 Feb 2023 16:44:26 +0000 (UTC) X-FDA: 80448326532.27.C845CBD Received: from mail-vs1-f46.google.com (mail-vs1-f46.google.com [209.85.217.46]) by imf13.hostedemail.com (Postfix) with ESMTP id AE60F20003 for ; Thu, 9 Feb 2023 16:44:23 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=crjYNly8; spf=pass (imf13.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.46 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1675961063; a=rsa-sha256; cv=none; b=ULjCuKO7e7rYV0440ef5v+dRSXaJQgPblmlmeYNH2G9eEkhatz+C3r6XX44BQMobVit3CJ tQ7r5nfz+uR9rsJD7rJx0L3yCJluLRq+ioBi/T6sqU45tjVKwAea0Ko8ijnWo94w43xYHQ cxOdKZnP87cDeXURGk0/9lm+pkJT+BA= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=crjYNly8; spf=pass (imf13.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.46 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1675961063; 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=Y6ufnwo9LJbhlFuZB107t4X1vUwtzNYzWsNbKQX/S5I=; b=v/UzgxCJ86w4m4aHNuk5Q0GDV+XbKWNt2LMJtAh7C48IhbTKckOwGaKAdxRGFY+SRpbArk WZplkH/AK1AXgLXudykMUQMJSlZa+wFSPkI71+yvxtf5h2Y1B6p92s3YdIp/FTu0DOT3rr JXY88sUbTl4cCAiJihFOyRozGa8EZcM= Received: by mail-vs1-f46.google.com with SMTP id h19so2706274vsv.13 for ; Thu, 09 Feb 2023 08:44:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1675961063; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Y6ufnwo9LJbhlFuZB107t4X1vUwtzNYzWsNbKQX/S5I=; b=crjYNly8goKM1PL6jvQqoL242bnk0fPHB2s/OWy8VaNbVIojCiZjCJ72qunaS77hrF 62PaK05FPoMs0Z/zyQDTxf8K9eHUyi0WJJKLCEfPJKNQ9455S+YcrMnY5ardc//tn3Kd cDOWU50PkMx2juUsPrGriXu3DhubDoeoGJINzUnkQLJQdZPK+UlPklS2CxebX3KPhx3c jbz6UaTtwWFjE+fgjiRQy5AyjsISUbvwbi6+icMqHTDZj2NQWY9yQeidrgHajXko3msd 69tkkc3IubSQxvRQT/uxkedYX5sJxoqsk8ToGROusER2IijIQhgd8FYDGKQWEK+m/NLc rTPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1675961063; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Y6ufnwo9LJbhlFuZB107t4X1vUwtzNYzWsNbKQX/S5I=; b=OObbb16DFwsUSPJyqS2+SFr1frg7fMWLT+EO0JnyYy4bVyQBM85af2t38y+KCEcpCq P4Y4IZ8NAJFQcMjYTNu0iE38GUs3vW9Bnpn76RTXACzyyHkkNTF8eyJPp+W9F2rBx6b/ ZgM/TsCabR2WKVFszFq2sXMEPualm+VdON9TOlZXn0wDKbh+0Mh075mej9SsrSy1TViQ CD5+nY/XoRNr1eAnP15bsoQpo96tGNKvEcLjAcb0COvG0rGMClfWCZCiVNRe6GgjkPdu TTHwMZJhT8r3u58zI/DZwMUf/uplrmihlpt/3bHyzWUnwK3pX1ps+gGkEImMVuRbysSj vuDA== X-Gm-Message-State: AO0yUKUjyx6xbSrxgdd/dhrBwbHYS+0zQapoSCaj/iJIysCyEYm1coiM qStnytiuQzXbLpaZHU7NfDA5jXVRr11pEUhSZ4Mcow== X-Google-Smtp-Source: AK7set897AhvKSt608HI0HaJ7+dgNerjiq1J9RXlB0xxyeyPMC5N1Q8qnLyWoTIXdyKuaGPi18NqibYej1lkdNzoZnk= X-Received: by 2002:a05:6102:1592:b0:411:adb8:83f with SMTP id g18-20020a056102159200b00411adb8083fmr1760047vsv.15.1675961062680; Thu, 09 Feb 2023 08:44:22 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: James Houghton Date: Thu, 9 Feb 2023 08:43:45 -0800 Message-ID: Subject: Re: [PATCH 21/46] hugetlb: use struct hugetlb_pte for walk_hugetlb_range To: Peter Xu Cc: Mike Kravetz , David Hildenbrand , Muchun Song , David Rientjes , Axel Rasmussen , Mina Almasry , "Zach O'Keefe" , Manish Mishra , Naoya Horiguchi , "Dr . David Alan Gilbert" , "Matthew Wilcox (Oracle)" , Vlastimil Babka , Baolin Wang , Miaohe Lin , Yang Shi , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Queue-Id: AE60F20003 X-Rspamd-Server: rspam01 X-Stat-Signature: uqid8fze9unegygryp6cjyei89oskaqe X-HE-Tag: 1675961063-860532 X-HE-Meta: U2FsdGVkX1+x6+KtEyZB+iL4b1M3XncWpxYuFu95gW9Yhr9A6VvK32elrNw0ke0sbTdFNynqWbTTA5rdXkNCcZ0JNF21lwxI43sMjb0kp5BCLggM2w39V+aM+Hb0xCGEU2+pVlOswPhEw7cqhkpYLxwMBDvzwF+xpv+J7JgIz0V62UG7sKG3SlBuLtO0Idnj8MXP9vdfzeSWrHaMmW6mnxGTi4RRAKu2RFWYv14Y4FUpHV4BDoOcHXBaR7iovTPAdmNbMkAjQx2/Y8rWdGzSfVgIc8XZ/nbsf125krWLVj8WlQe/tV1pAiHvGFeMhH4ZS46wna0EMudi572q/scb63KHuKKIJOV9OnkXI8GsCIddpjCYA37FsPiLl3N+r8tx2w3eZIr1W+/VlUL5P2a+d48alWhSYAo350Y9E2wtSSwrV+f/+1BJSgV4rjhy/yD4EgHkJP6tWgxOWPNL78wtF7E4XSwp8jgJtdEI90++B1N/7Q7QOSHAUnNKvFQRRUV5ilpmLmjnuYAxnwT5SDUKwc4QkoNNE6LS/V7iKfgBxDP/Ms1geD4A3HhDYiRePbOfHnyNSZbNRKzvH7lpiEHNR/op+iWZ0aW1JtN7vNrON36CMxE/6+r7gsE75YnG4x2JjDBGxiwyOleAeyPjk9t3MmGJnfwRow/HN3YmGjEy90NyytLGEV9Q3MwQ+VluR4rgZlN4tcmvQTzuYNHIKat8H01cpJwhK/qKSToLlqNGXZNPEi1UTHuonAWdDE9s2SsZv+JOckkre0neZPqgqjIJ1u3EnUw0zqwDll6AO4U9+5VjnM/AZenXPUcXbyWqgm4ULGqV45XqnFNi4qYyrnbH8E8HJZgx0KniGQqyuhsvl1EQpxfhf9GGkO5Onh4yZ1zo7hZbP8/o+7IAYiudCikWcOvl9klrsJ01HJi1Lj4xXzNI7fEy3OuYOZfzJqCSTMBkAP9pNeA6qZ0/m2AY9/l MgiPI5Z8 dtBS62CrCK42CyQc54orGtKkAL0lP0O4lPsynDadFOaTdTqspKZBAc3L/dqWMoLnz4NtVXQGB7gUR7C5pJY89XnT5RARvRkDT28Torh5Hvt5xWEqYbQy3ikquyCxycxgC2Gx4XFTe1FsUF0xJo2yr/pKVxCJNClLjIvPFmyJimJw06Q0D322tPU04jxF236sUL/Wrb3FttoYJ+mV0ismPQ1MQ+40aFjOUXstIbgXAhuMxk7Cf/wAtrgVP9iwo6U35z5En+yj3jl3rMt/O8E02mVgwSe5K+dCK/q+nYp5NoUPXVKBfBEOX2PtUu3jP+AD8TdtE 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 Wed, Feb 8, 2023 at 8:16 AM Peter Xu wrote: > > On Tue, Feb 07, 2023 at 04:26:02PM -0800, James Houghton wrote: > > On Tue, Feb 7, 2023 at 3:13 PM Peter Xu wrote: > > > > > > James, > > > > > > On Tue, Feb 07, 2023 at 02:46:04PM -0800, James Houghton wrote: > > > > > Here is the result: [1] (sorry it took a little while heh). The > > > > > > Thanks. From what I can tell, that number shows that it'll be great we > > > start with your rfcv1 mapcount approach, which mimics what's proposed by > > > Matthew for generic folio. > > > > Do you think the RFC v1 way is better than doing the THP-like way > > *with the additional MMU notifier*? > > What's the additional MMU notifier you're referring? An MMU notifier that informs KVM that a collapse has happened without having to invalidate_range_start() and invalidate_range_end(), the one you're replying to lower down in the email. :) [ see below... ] > > > > > > > > > > > implementation of the "RFC v1" way is pretty horrible[2] (and this > > > > > > Any more information on why it's horrible? :) > > > > I figured the code would speak for itself, heh. It's quite complicated. > > > > I really didn't like: > > 1. The 'inc' business in copy_hugetlb_page_range. > > 2. How/where I call put_page()/folio_put() to keep the refcount and > > mapcount synced up. > > 3. Having to check the page cache in UFFDIO_CONTINUE. > > I think the complexity is one thing which I'm fine with so far. However > when I think again about the things behind that complexity, I noticed there > may be at least one flaw that may not be trivial to work around. > > It's about truncation. The problem is now we use the pgtable entry to > represent the mapcount, but the pgtable entry cannot be zapped easily, > unless vma unmapped or collapsed. > > It means e.g. truncate_inode_folio() may stop working for hugetlb (of > course, with page lock held). The mappings will be removed for real, but > not the mapcount for HGM anymore, because unmap_mapping_folio() only zaps > the pgtable leaves, not the ones that we used to account for mapcounts. > > So the kernel may see weird things, like mapcount>0 after > truncate_inode_folio() being finished completely. > > For HGM to do the right thing, we may want to also remove the non-leaf > entries when truncating or doing similar things like a rmap walk to drop > any mappings for a page/folio. Though that's not doable for now because > the locks that truncate_inode_folio() is weaker than what we need to free > the pgtable non-leaf entries - we'll need mmap write lock for that, the > same as when we unmap or collapse. > > Matthew's design doesn't have such issue if the ptes need to be populated, > because mapcount is still with the leaves; not the case for us here. > > If that's the case, _maybe_ we still need to start with the stupid but > working approach of subpage mapcounts. Good point. I can't immediately think of a solution. I would prefer to go with the subpage mapcount approach to simplify HGM for now; optimizing mapcount for HugeTLB can then be handled separately. If you're ok with this, I'll go ahead and send v2. One way that might be possible: using the PAGE_SPECIAL bit on the hstate-level PTE to indicate if mapcount has been incremented or not (if the PTE is pointing to page tables). As far as I can tell, PAGE_SPECIAL doesn't carry any meaning for HugeTLB PTEs, but we would need to be careful with existing PTE examination code as to not misinterpret these PTEs. > > [...] > > > > > > Matthew is trying to solve the same problem with THPs right now: [3]. > > > > > I haven't figured out how we can apply Matthews's approach to HGM > > > > > right now, but there probably is a way. (If we left the mapcount > > > > > increment bits in the same place, we couldn't just check the > > > > > hstate-level PTE; it would have already been made present.) > > > > > > I'm just worried that (1) this may add yet another dependency to your work > > > which is still during discussion phase, and (2) whether the folio approach > > > is easily applicable here, e.g., we may not want to populate all the ptes > > > for hugetlb HGMs by default. > > > > That's true. I definitely don't want to wait for this either. It seems > > like Matthew's approach won't work very well for us -- when doing a > > lot of high-granularity UFFDIO_CONTINUEs on a 1G page, checking all > > the PTEs to see if any of them are mapped would get really slow. > > I think it'll be a common problem to userfaultfd when it comes, e.g., > userfaultfd by design is PAGE_SIZE based so far. It needs page size > granule on pgtable manipulations, unless we extend the userfaultfd protocol > to support folios, iiuc. > > > > > > > > > > > > > > > > We could: > > > > > - use the THP-like way and tolerate ~1 second collapses > > > > > > > > Another thought here. We don't necessarily *need* to collapse the page > > > > table mappings in between mmu_notifier_invalidate_range_start() and > > > > mmu_notifier_invalidate_range_end(), as the pfns aren't changing, > > > > we aren't punching any holes, and we aren't changing permission bits. > > > > If we had an MMU notifier that simply informed KVM that we collapsed > > > > the page tables *after* we finished collapsing, then it would be ok > > > > for hugetlb_collapse() to be slow. [ from above... ] This MMU notifier. :) > > > > > > That's a great point! It'll definitely apply to either approach. > > > > > > > > > > > If this MMU notifier is something that makes sense, it probably > > > > applies to MADV_COLLAPSE for THPs as well. > > > > > > THPs are definitely different, mmu notifiers should be required there, > > > afaict. Isn't that what the current code does? > > > > > > See collapse_and_free_pmd() for shmem and collapse_huge_page() for anon. > > > > Oh, yes, of course, MADV_COLLAPSE can actually move things around and > > properly make THPs. Thanks. But it would apply if we were only > > collapsing PTE-mapped THPs, I think? > > Yes it applies I think. And if I'm not wrong it's also doing so. :) See > collapse_pte_mapped_thp(). > > While for anon we always allocate a new page, hence not applicable. > > -- > Peter Xu Thanks Peter! - James