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 6E3C1C25B7F for ; Wed, 16 Aug 2023 21:31:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 50A3428001A; Wed, 16 Aug 2023 17:31:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4BA96280016; Wed, 16 Aug 2023 17:31:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 35B2828001A; Wed, 16 Aug 2023 17:31:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 23F85280016 for ; Wed, 16 Aug 2023 17:31:47 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id BFBA11C9658 for ; Wed, 16 Aug 2023 21:31:46 +0000 (UTC) X-FDA: 81131265012.09.A47CC7B Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by imf16.hostedemail.com (Postfix) with ESMTP id AAE7818001E for ; Wed, 16 Aug 2023 21:31:44 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=TxeYRwWD; spf=pass (imf16.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=zokeefe@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692221504; a=rsa-sha256; cv=none; b=KEB2IExL7a5ci0u7JM2Hbdvi7Odjz0aaLQAXtdJroTHddUM5U8rk5R833aMyPU07+bq1no UMjR/bilKTsvpoaninJQVRPW8He26C7duXjguXxwrJmFvOKjeC/iVpDPwspzhot+3wbY2N 4AWavM47tvtRYFNYU3yf8CQESaMXNbY= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=TxeYRwWD; spf=pass (imf16.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=zokeefe@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=1692221504; 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=4jpiFyBqyB7GEQ4UWrE0SW3sqFRGgppX7U6JuEt4HtU=; b=W+s8AhPhT4t3uOwnCiX8kcqwjegQXAtsWDEWHnawnjkTmB+nKFw4LoJfHeMAgcFN3fAeGz OQGjchrVJEJ1dH3EDKqBQM7Z6zXIibWhgPCLFGB9fxrmmfzkdIMQw/MAB0/D8T3F5odgwv +0nVVtboB1pclH0ToNGQs2S8IepDMWI= Received: by mail-ed1-f51.google.com with SMTP id 4fb4d7f45d1cf-5257d67368bso4073a12.0 for ; Wed, 16 Aug 2023 14:31:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1692221503; x=1692826303; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=4jpiFyBqyB7GEQ4UWrE0SW3sqFRGgppX7U6JuEt4HtU=; b=TxeYRwWDVYHfd9uxbK3fhjziQdOpSjFLnx5FCZR3j+hN0ph/TWOrQT7d+PHOR8QWym gEXrZIVyvoufGajBIXvj13/i7rJoRiIyyRbgvU1fzJk3DRt0x+QzaMzcgkCOWNf40m9T 37B8+e9L7cB1/anxay/UpKznyy4/m2EGeSaTcksjSr78wqQ+Cg3h4d+db2llRHDUVzzv lXxdbhR5IENQ0nCIYihWR15VPzKAP+FsDzR2TGeKbPZQZB/yihavoyM9+TkBaQaXDePr +0yQcXqV46hqrHHs543eaDwZuF1WBNa6jyjbky9Re4cWQ5b+vq3HtJ7h01IBrnKd+T+S /TZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692221503; x=1692826303; h=content-transfer-encoding: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=4jpiFyBqyB7GEQ4UWrE0SW3sqFRGgppX7U6JuEt4HtU=; b=eSI7O64qTR3aYTGpXqbujyLIbV7P4ZEeolhRvNMOMA03CbArijkrJiemzDvz82cSHb mk3W6AjhkCkxxCFLb1feQeEubPNwm308iIqykoSJJJu24wBZQs7Vy9UOg91uzuQrkqQR /PME6Ij+mh+Bz1hMV/hoowsg5SW4ao272Rv34GHdZYCgFqg9EOl1/wU/oh/OKOqdPHan SOQUxdBDCHXbYSnkG+DnHJ35GJfuepcQJ3FEu+F7RJRagiZ6pbGdscs0TY6vKik5vS8I 4nNYCx7ZBwL+Bt/BG4CCA9HDwDUzBK3iiKkhl10NtG2E1aEjL1aUGCQwwdQETllWEAfk iyew== X-Gm-Message-State: AOJu0Yy0ybrPw0axnYuksIWLQaBr1lscUjtc/Yo6L1LbHwGkFjH5HMpV /dUWeL7s3Waem2An3aWyA5+QB34W3uJyUCz6LOmEjw== X-Google-Smtp-Source: AGHT+IHI2ZvcVIAINadCqQ5irpfw4urDyIeFJW7lkcFNTkgk8/0tcrZmU7TZsQ2BfRehvcruporPMLnB/cRrubjUCzI= X-Received: by 2002:a50:9fae:0:b0:522:28a1:2095 with SMTP id c43-20020a509fae000000b0052228a12095mr9721edf.3.1692221502915; Wed, 16 Aug 2023 14:31:42 -0700 (PDT) MIME-Version: 1.0 References: <20230812210053.2325091-1-zokeefe@google.com> In-Reply-To: From: "Zach O'Keefe" Date: Wed, 16 Aug 2023 14:31:06 -0700 Message-ID: Subject: Re: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill __transhuge_page_enabled()" To: Matthew Wilcox Cc: Saurabh Singh Sengar , Dan Williams , "linux-mm@kvack.org" , Yang Shi , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: AAE7818001E X-Stat-Signature: p7mrdfnkn9fxnckfewhtax59wg3ixpt1 X-Rspam-User: X-HE-Tag: 1692221504-636536 X-HE-Meta: U2FsdGVkX1+HdbDrW17q+KMSXbrlqMWgfORekMnc+SwW3jWBfuHqbxF5LK+EH+/jwEdZqQaf1we+Hvtn+I0nPnhB7Ue7ruNkL7vTPSp09XEci3tEvnJoXNqu9q9n4U9ncVnBWNcDNOjVllswgW+fLMVGR5nxolniUx/5AU9j9EAA1TtDWpskLcf86aoh32+cyTWuzwcPwdWg/0OQ8y48VXtssPNXfxmXeQuuZGANLMLh9sVT0lR+/DWf7A7vDNKDPIToYVOhf5BK6j+GgQs3zRHQNd23YEGtNp5dHTm3A0dp6Z85dys40xNJu8WaXXHsC3v6J03HFxLWZqsnP18w9b4na8oxinI3tJbgFNrgxPAIvqVvBkod0YRK2ts+js+Igz7zr3Nr8fu37ztKN1bXG2SJZmVVXUt7cLI2wQUnMVJS1+zJw0nqY0ezR657dNwzxXz2SD815HS+dfn6y1rFCYNJ4XBxCbPb6xuUzj2tFUusFOuCXWAo3pQ7jVmdYnCDhS73uzCs8wZv6OBoPW5uCdI3sj8H4Prjd+hOiHAbaBSjfLr5be25OvkozpnX8KxXu1zm9iJMR9dabFv7v6QZjVzp1mg3cWHqRYOO5hHgHH9GQmXBXtfMv9yU4cUtm63n9u+g7ghAFxyobvNwTySJ653A3vdkLJbOL0xUSJc2x9jP9WXa+3n+qmDchUdxP0tfipw5KDjwb5TobzJaII7mdtQ5jWOg/X7U8N+3GKcP84/gYJwf9jQsNktPplLdCHUWhLZpqSxD7vx+E9tRQihgUO910SC3FEmmz2wXBrZzk79k71fsmH89PLsXj3DBzck7ZkwVrz/EjDBgo9EXRw26brFGD86cok9lef6dwhYLFGFqWjo59/9ZqQdwn/8cBQ2TW1YJfGOJEuIqKpJ9kNGDI7a8aVcs/yTCvEVdaToGwFCCg6AmC0bWtC3FY8evkzIXdCcX/XKACk0s2onZZI2 70apcu+o YHGoBvgcNjSmmwAqZsQCCG3Mzsi8hn1yK5heG0xPyc577dUSCLKhgqcrJvjVwJ8F9ydKYI0F0Qf+0QQbY87zkYZhgSBQBChqjSAeZCaQKDk9M+zwSEiBYoCo8wF8woT54JNErV7xpk3UP6imiQzI88M+FplBNsbusK/MfD6RdWXQKbbZMJzOGI7zYK6JvHGyswenEosvbsp9mBLtrF+YG19BWpaq5ps1lYkevnQc3/Oc9/SzJBeY7R6UjK8XZ1VoSwiFrxN00LZ6HGf6NvKB4PKLNr+8vrq4RcZD9mtcnhmanW8deqylw19J4KHFEZ0aG7pemvfwIHPIExT82dLhL6eW/zndB1yDFqlyJLbQgdJ8e5KDVFjt61YDDjQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000007, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Aug 14, 2023 at 7:24=E2=80=AFPM Matthew Wilcox wrote: > > On Mon, Aug 14, 2023 at 05:04:47PM -0700, Zach O'Keefe wrote: > > > From a large folios perspective, filesystems do not implement a speci= al > > > handler. They call filemap_fault() (directly or indirectly) from the= ir > > > ->fault handler. If there is already a folio in the page cache which > > > satisfies this fault, we insert it into the page tables (no matter wh= at > > > size it is). If there is no folio, we call readahead to populate tha= t > > > index in the page cache, and probably some other indices around it. > > > That's do_sync_mmap_readahead(). > > > > > > If you look at that, you'll see that we check the VM_HUGEPAGE flag, a= nd > > > if set we align to a PMD boundary and read two PMD-size pages (so tha= t we > > > can do async readahead for the second page, if we're doing a linear s= can). > > > If the VM_HUGEPAGE flag isn't set, we'll use the readahead algorithm = to > > > decide how large the folio should be that we're reading into; if it's= a > > > random read workload, we'll stick to order-0 pages, but if we're gett= ing > > > good hit rate from the linear scan, we'll increase the size (although > > > we won't go past PMD size) > > > > > > There's also the ->map_pages() optimisation which handles page faults > > > locklessly, and will fail back to ->fault() if there's even a light > > > breeze. I don't think that's of any particular use in answering your > > > question, so I'm not going into details about it. > > > > > > I'm not sure I understand the code that's being modified well enough = to > > > be able to give you a straight answer to your question, but hopefully > > > this is helpful to you. > > > > Thank you, this was great info. I had thought, incorrectly, that large > > folio work would eventually tie into that ->huge_fault() handler > > (should be dax_huge_fault() ?) > > > > If that's the case, then faulting file-backed, non-DAX memory as > > (pmd-mapped-)THPs isn't supported at all, and no fault lies with the > > aforementioned patches. > > Ah, wait, hang on. You absolutely can get a PMD mapping by calling into > ->fault. Look at how finish_fault() works: > > if (pmd_none(*vmf->pmd)) { > if (PageTransCompound(page)) { > ret =3D do_set_pmd(vmf, page); > if (ret !=3D VM_FAULT_FALLBACK) > return ret; > } > > if (vmf->prealloc_pte) > pmd_install(vma->vm_mm, vmf->pmd, &vmf->prealloc_= pte); > > So if we find a large folio that is PMD mappable, and there's nothing > at vmf->pmd, we install a PMD-sized mapping at that spot. If that > fails, we install the preallocated PTE table at vmf->pmd and continue to > trying set one or more PTEs to satisfy this page fault. Aha! I see. I did not expect ->fault() to have this logic, as I had incorrectly thought (aka assumed) the pmd vs pte-mapping logic split at create_huge_pmd(); i.e. do_huge_pmd_anonymous_page(), or ->huge_fault(), or fallback to pte-mapping. It seems very weird to me that hugepage_vma_check() "artificially" says "no" to file and shmem along the fault path, so they can go and do their own thing in ->fault(). But this has been the way non-anon has supported THP fault since the start ... with Kiril's commit 4.8 commit 101024596441 ("mm: introduce do_set_pmd()") as part of the original "THP-enabled tmpfs/shmem using compound pages" series. I just did not know about it :/ But thanks for prompting this -- I learnt a lot reading further down do_fault(). shmem's ability to fault THP will depend on how the file was constructed to begin with (i.e the huge=3D mount option). For file, our ability to pmd-map the folio will depend on if the folio was assembled in the pagecache as a large folio or not -- which depends on the fs' AS_LARGE_FOLIO_SUPPORT (for which, only xfs, erofs, and afs support today). I tested things on xfs, and it does actually work. Cool :) IIUC then, there is a bug in smaps THPeligible code when CONFIG_READ_ONLY_THP_FOR_FS is not set. Not obvious, but apparently this config is (according to it's Kconfig desc) khugepaged-only, so it should be fine for it to be disabled, yet allow do_sync_mmap_readahead() to install a pmd for file-backed memory. hugepage_vma_check() will need to be patched to fix this. But I have a larger question for you: should we care about /sys/kernel/mm/transparent_hugepage/enabled for file-fault? We currently don't. Seems weird that we can transparently get a hugepage when THP=3D"never". Also, if THP=3D"always", we might as well skip the VM_HUGEPAGE check, and try the final pmd install (and save khugepaged the trouble of attempting it later). WDYT? Aside: should have brought this to Cabal meeting today, but hadn't finished going through things > > So why, you may be asking, do we have ->huge_fault. Well, you should > ask the clown who did commit b96375f74a6d ... in fairness to me, > finish_fault() did not exist at the time, and the ability to return > a PMD-sized page was added later. > :P But that patch seems super reasonable? At least my naive initial reading assumed exactly what that commit description says.