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 6CD84C05027 for ; Wed, 1 Feb 2023 17:59:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C8DB46B0072; Wed, 1 Feb 2023 12:59:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C3D356B0073; Wed, 1 Feb 2023 12:59:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B058A6B0074; Wed, 1 Feb 2023 12:59:07 -0500 (EST) 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 9ED276B0072 for ; Wed, 1 Feb 2023 12:59:07 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 6A2121A0DC1 for ; Wed, 1 Feb 2023 17:59:07 +0000 (UTC) X-FDA: 80419484334.07.D3F2DB3 Received: from mail-vs1-f53.google.com (mail-vs1-f53.google.com [209.85.217.53]) by imf11.hostedemail.com (Postfix) with ESMTP id B007A40011 for ; Wed, 1 Feb 2023 17:59:05 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="rh4iOj//"; spf=pass (imf11.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.53 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=1675274345; 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=cPFfYuyib9emhUUABsRMDshvSQtpvkW/9z+CFnKeyvA=; b=ldc67UgMhfeZPxXlIxEB2JKGpZHY78bn0RAWLsjvp50XAl/gA31VKtGZf9MQbDr3VmtYPN S1eZqIeUkH4vCvSna09YLqPB+jeMM0Je00DFylfLgKTSzqHYUNqnZCoItktrbAHFtkn7ik e1ZKx5T3LjrPdr0MxWKSOuDJ+ejEQGU= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="rh4iOj//"; spf=pass (imf11.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.53 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=1675274345; a=rsa-sha256; cv=none; b=a7GfmLnfrclRfV/e5MWkqCC2rQ7ZHzqDXhpsv4GHOF+Qbev3y3ZjucYh1XTQKwR4GcZqkL mJ2ukv64vu/esbOKosnRGeFYgMelMWyfw16tfD4ahcECyCsDKx1mYZa3Qhwh4bmAQrMsv+ Z5AbsReROpy0vbaSz97ZHKEyhiAJ9m4= Received: by mail-vs1-f53.google.com with SMTP id 187so20520151vsv.10 for ; Wed, 01 Feb 2023 09:59:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=cPFfYuyib9emhUUABsRMDshvSQtpvkW/9z+CFnKeyvA=; b=rh4iOj//EViSBgmMXraaxnVrS7aQUuct5v/JQggXExkCBiIrUBfB/wBG8GSQhA2rfL 56BWRpdpjBpnx8FjpOK2cR8RgQHkUgfgZevn4PrBrc4mla8JOo5VbpIfw/TwNGBqB4g5 NJ0GkLi9SvCZ30Lboa3RYZrUEuAR+CWW8gKc36f4Tl6ih1BAB8vwlXYZ2ovZ3M6cTtRM t78dC9D7GgUvbX272ejErN8ALQR8KN2eg6hXtsI5V7YbWtVE5aKVuKZpwsLGbFhtwKK2 hLRq1rzEuUtEsG7hydLDZHk7KqXPntE1Y/EPIaxAQWbmq5NLjyvyLz3ZrDL/G1Qnw/w1 w8nw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=cPFfYuyib9emhUUABsRMDshvSQtpvkW/9z+CFnKeyvA=; b=RrRJn6Bjl2zoQNSmcch6PIgR4b37P4cJNSiFGsk+2Q8m9N6NapEV7XCOngmOgc4Nnr X/7Hqihos6NMR4b8zZ+ja6aFqKN8JmeTZF93rfR+rSU4fC94K5A65oaLb0pqgKGLOmcM 80CIzo+A2gTmxpkyz+tA10Wl2TbE1zA2HCLD29O/0eEPT6v4Irtk5+Ka+Syg2FDvjrcg H7zAdqOkXJ3MaTPV/TkUHnvs1cM+NFVPtUaUtHiM7fnUaPe53CYE3Zh3aXN/g2Y9W6dV ws63sK0CtwMOJkrp6Eg4p6Y0OBjXUkFXL1c8BzVygeg3SQ288jJJmHAJM1u3mm+hqpdh 66Aw== X-Gm-Message-State: AO0yUKULfaq6xrG0BAr17m3gom2aYRBYJ5I8u71kexFj2j+EQTr9cvAW 31ChG+5PTA1w6en/4qJ1dV/xJg7d7wJO4onY9KtVmA== X-Google-Smtp-Source: AK7set8IO/rVKPm/kvAT1ID0Rhcp8vh6DVh2oh2sEO4whdwrRXgDKzRppiwa7e5hGgyQzT5cvqgvJjZhxhOVd7T0/zM= X-Received: by 2002:a67:d984:0:b0:3f9:9403:6000 with SMTP id u4-20020a67d984000000b003f994036000mr572591vsj.1.1675274344768; Wed, 01 Feb 2023 09:59:04 -0800 (PST) MIME-Version: 1.0 References: <7a1bc3c5-6efe-87ab-1276-f71fc440c821@redhat.com> In-Reply-To: <7a1bc3c5-6efe-87ab-1276-f71fc440c821@redhat.com> From: James Houghton Date: Wed, 1 Feb 2023 09:58:27 -0800 Message-ID: Subject: Re: [PATCH 21/46] hugetlb: use struct hugetlb_pte for walk_hugetlb_range To: David Hildenbrand Cc: Peter Xu , Mike Kravetz , 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-Stat-Signature: ywc6zjgnibamihcjdqgfub7p7oiqypnr X-Rspam-User: X-Rspamd-Queue-Id: B007A40011 X-Rspamd-Server: rspam06 X-HE-Tag: 1675274345-316837 X-HE-Meta: U2FsdGVkX1/ItnERImSrN3tit2QAVCkNh8mmd+in5eqewyGSs3zzs40v4RPxzeBLhgLViqxiRJv1gHyBEP+7UFt0P2pl2AF6xrxEMaCm/YLkMfV1ZiRqMmzG8eh10lwpxVxGslcNEAr2ik9aPlTb41cz6OxTA/LcnPL8Vu6oPBNNXaSwC8sGMxDtG3o57J2KUFzyB68PorO9VrYji7XjO0EsubLb3/5WsDO9p4V9BDsvxNtSLvrSjrzZRrTpPXAHhNWU8DCJuqtBLqS6837Ee/Vzy2FNSMsuLQ5lU3d0G3EJCLwZP9bjVF0HQzidpR7/mao+ip7utjM9+6U8w2xyYrRb52mg4KvyW6mX3wB3O7LEatXPPclb0l6HTGxPHJ5El5TCxWURkHD6YbuZN081zKOzbYh0O9WJC86qHYD78nSVET8XS2+FP+KzAU10bETNmSukacH+GV33+Dqm/SczT5rOBqy7Y4J8o/tg+BLON7vSUY1aOPKfyAZzuk3RB3OcG3eh7DL4a0qmEmoyFgDrTtfy6dxZIKyyBmwgmwBYN/HXugmipuHIUU3dMF1KIon0p4HLkbDibCayki1KGLJEAIxbYkr/r9M+pn+UPLek11Yd3cfeAe5Zzn/EuYfVOQg8bJnmtfB8b0MKEYmrY+A3NFafqwV3quEWwiTnmkV+wEgnAjilmPeVLti1rtDh60EKD8CBtgjsKcl89JbOu+lcC224P4nDP1nZPZ3eooF5w9EpYFL4M4bOYU3IfDQUc1a9pbyjX83z6arWJ0pGwE3zXAQmhIJaC3o5xtfulyN9YMF7HeLibpiYjnEpMBTgcHexcBBhy02Tuh2s4gGke+rcNIAJaukuZ2LsS/gnTXGqJ7XnH7uj9+yfDP6VI1EARMW8lk0Na9CIN4kkM2FcqQ3GoAvCiu1Rshv9vdiA/sbVLmR18pneZu8Qd/ZIiUJ1pKHxK/VIb1hnq1IQqp3bJfc otTCKA1u 9xCgER+4ARhyOwiKMkybySqrvZYYT2a/DMz2qQbQNhBVbZ7jzF2gPXDTzdLzs077oCjdKg0aoeAnTO19sV5s+4idse2aLhqtzIDlGaom/IefG9l4ABkvEWW9rDiVW01/swCqQkW7KHI4xEZ+n9ltjmzilGbiV0bcL7nKesbW2FebLw1vI69JeW4BipSYd8kdJisIDtiNR/rEdQoyAMeerYgJq4+Qc8fJwqMSCdoI+3SfwtdXEuwyG4EWSeJ44nlF1kGqp4llAdOdA2tqNKmPocpWhmTXVYuW4Apkzaq8lV4LARCZYIULDjVQ/4vLVhVBpFRi6OWu5VG8qhPh4IdkO2FCU6rrcWezFy2RX0PpQ7FOQ4JrXEFEs2s/WioCiTJjWSwAHIL9Hdx8pXliORNzZ0aIImvxHoj163WZTFZdtgYWWnFDRtXsyk6vBNQ== 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 1, 2023 at 7:56 AM David Hildenbrand wrote: > > On 01.02.23 16:45, James Houghton wrote: > > On Tue, Jan 31, 2023 at 5:24 PM Peter Xu wrote: > >> > >> On Tue, Jan 31, 2023 at 04:24:15PM -0800, James Houghton wrote: > >>> On Mon, Jan 30, 2023 at 1:14 PM Peter Xu wrote: > >>>> > >>>> On Mon, Jan 30, 2023 at 10:38:41AM -0800, James Houghton wrote: > >>>>> On Mon, Jan 30, 2023 at 9:29 AM Peter Xu wrote: > >>>>>> > >>>>>> On Fri, Jan 27, 2023 at 01:02:02PM -0800, James Houghton wrote: > > [snip] > >>>>>> Another way to not use thp mapcount, nor break smaps and similar calls to > >>>>>> page_mapcount() on small page, is to only increase the hpage mapcount only > >>>>>> when hstate pXd (in case of 1G it's PUD) entry being populated (no matter > >>>>>> as leaf or a non-leaf), and the mapcount can be decreased when the pXd > >>>>>> entry is removed (for leaf, it's the same as for now; for HGM, it's when > >>>>>> freeing pgtable of the PUD entry). > >>>>> > >>>>> Right, and this is doable. Also it seems like this is pretty close to > >>>>> the direction Matthew Wilcox wants to go with THPs. > >>>> > >>>> I may not be familiar with it, do you mean this one? > >>>> > >>>> https://lore.kernel.org/all/Y9Afwds%2FJl39UjEp@casper.infradead.org/ > >>> > >>> Yep that's it. > >>> > >>>> > >>>> For hugetlb I think it should be easier to maintain rather than any-sized > >>>> folios, because there's the pgtable non-leaf entry to track rmap > >>>> information and the folio size being static to hpage size. > >>>> > >>>> It'll be different to folios where it can be random sized pages chunk, so > >>>> it needs to be managed by batching the ptes when install/zap. > >>> > >>> Agreed. It's probably easier for HugeTLB because they're always > >>> "naturally aligned" and yeah they can't change sizes. > >>> > >>>> > >>>>> > >>>>> Something I noticed though, from the implementation of > >>>>> folio_referenced()/folio_referenced_one(), is that folio_mapcount() > >>>>> ought to report the total number of PTEs that are pointing on the page > >>>>> (or the number of times page_vma_mapped_walk returns true). FWIW, > >>>>> folio_referenced() is never called for hugetlb folios. > >>>> > >>>> FWIU folio_mapcount is the thing it needs for now to do the rmap walks - > >>>> it'll walk every leaf page being mapped, big or small, so IIUC that number > >>>> should match with what it expects to see later, more or less. > >>> > >>> I don't fully understand what you mean here. > >> > >> I meant the rmap_walk pairing with folio_referenced_one() will walk all the > >> leaves for the folio, big or small. I think that will match the number > >> with what got returned from folio_mapcount(). > > > > See below. > > > >> > >>> > >>>> > >>>> But I agree the mapcount/referenced value itself is debatable to me, just > >>>> like what you raised in the other thread on page migration. Meanwhile, I > >>>> am not certain whether the mapcount is accurate either because AFAICT the > >>>> mapcount can be modified if e.g. new page mapping established as long as > >>>> before taking the page lock later in folio_referenced(). > >>>> > >>>> It's just that I don't see any severe issue either due to any of above, as > >>>> long as that information is only used as a hint for next steps, e.g., to > >>>> swap which page out. > >>> > >>> I also don't see a big problem with folio_referenced() (and you're > >>> right that folio_mapcount() can be stale by the time it takes the > >>> folio lock). It still seems like folio_mapcount() should return the > >>> total number of PTEs that map the page though. Are you saying that > >>> breaking this would be ok? > >> > >> I didn't quite follow - isn't that already doing so? > >> > >> folio_mapcount() is total_compound_mapcount() here, IIUC it is an > >> accumulated value of all possible PTEs or PMDs being mapped as long as it's > >> all or part of the folio being mapped. > > > > We've talked about 3 ways of handling mapcount: > > > > 1. The RFC v2 way, which is head-only, and we increment the compound > > mapcount for each PT mapping we have. So a PTE-mapped 2M page, > > compound_mapcount=512, subpage->_mapcount=0 (ignoring the -1 bias). > > 2. The THP-like way. If we are fully mapping the hugetlb page with the > > hstate-level PTE, we increment the compound mapcount, otherwise we > > increment subpage->_mapcount. > > 3. The RFC v1 way (the way you have suggested above), which is > > head-only, and we increment the compound mapcount if the hstate-level > > PTE is made present. > > > > With #1 and #2, there is no concern with folio_mapcount(). But with > > #3, folio_mapcount() for a PTE-mapped 2M page mapped in a single VMA > > would yield 1 instead of 512 (right?). That's what I mean. > > My 2 cents: > > The mapcount is primarily used (in hugetlb context) to > > (a) Detect if a page might be shared. mapcount > 1 implies that two > independent page table hierarchies are mapping the page. We care about > mapcount == 1 vs. mapcount != 1. > > (b) Detect if unmapping was sucessfull. We care about mapcount == 0 vs. > mapcount != 0. > > For hugetlb, I don't see why we should care about the subpage mapcount > at all. Agreed -- it shouldn't really matter all that much. > > For (a) it's even good to count "somehow mapped into a single page table > structure" as "mapcount == 1" For (b), we don't care as long as "still > mapped" implies "mapcount != 0". Thanks for your thoughts, David. So it sounds like you're still squarely in the #3 camp. :)