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 9B013CD4846 for ; Fri, 22 Sep 2023 16:57:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 19FCF6B02EA; Fri, 22 Sep 2023 12:57:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 151766B02EB; Fri, 22 Sep 2023 12:57:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F33236B02EC; Fri, 22 Sep 2023 12:57:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id DC1936B02EA for ; Fri, 22 Sep 2023 12:57:02 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 9ABD3C0266 for ; Fri, 22 Sep 2023 16:57:02 +0000 (UTC) X-FDA: 81264838284.28.F960688 Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) by imf03.hostedemail.com (Postfix) with ESMTP id B4E1220003 for ; Fri, 22 Sep 2023 16:57:00 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=lQkxVXlc; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=zokeefe@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695401820; 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=7acKa9vg/VWzTL8BaOiKje9AL2xB6Xnf1TelKGJcqls=; b=O0gWAXXX6xdyDaGilroICMEEtQb+vwd3D9nYktvYHJvK05l7UO8D1TAJ8V85td3DLgz60+ TPoXRQfgmTZ7oHFbKSB9VByW/cybQYhd2nOuyMDb6lxrBg6+O/JXTG1HtL+xk2Uz2AVloP y4ScFd4PNde7s36eRIMKwOGFpH6V7lE= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=lQkxVXlc; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=zokeefe@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695401820; a=rsa-sha256; cv=none; b=gnyaB/kiRVDmdqDPkQI+Hn/lkfvISIAI2yK3Q9Z5ClEoeBhZXcjDvACh1eHZ1Ri44XFVTs qC9RABHRIK7tphaQlvTnFqzjB783i/VFW9ZaehC8UkCPbE4DSHHVM6t689T6q8LwycpeVJ qESTv9ZFGwgPgJnB59r0fe79GTRUBKI= Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-51e24210395so532a12.0 for ; Fri, 22 Sep 2023 09:57:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1695401819; x=1696006619; darn=kvack.org; 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=7acKa9vg/VWzTL8BaOiKje9AL2xB6Xnf1TelKGJcqls=; b=lQkxVXlcvcXcRccENt1zmx08xjZH/w8Z3a4TCdAyf+thwg1ra/hghU+R3FZwAJU24Y nTiNOLcg9r3BIPCVJzPQDza1UuIk1u9jZLXgJxTD4XmAHrgEsfoYnI5MTtCL2RaL+sUU AbQQCVNKvPcDhbicLF05y80VbJ+eMQizFH6tiD4DiCdSMXtKTKYBVN+8VnO4gYD/2PDk Wiif2nYU9Cp+wpuBZ/iGj2y0eK1bNQcyJHvbskZb5BxTzCuncZkgCxXiy06z4mcZalfG Gx9dR4qBHrIfZVKkLWtD7ZbShkIotPdnVSeZ0I2iiiSnw/ImsxPxyt24mNbCHoJbFkeG NawA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695401819; x=1696006619; 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=7acKa9vg/VWzTL8BaOiKje9AL2xB6Xnf1TelKGJcqls=; b=AuxU6IaaAr/I4Og2GUk1uftl+O5zcKWdF1WiQLFGofDZ4NWnyvRqnc80h4Lx4uLFrk 9ZO4l0QebZXdKPOwvkMaiJk/GjYJHtU3jDAzahOReueV+wsm7CbvuNR9DAF1ctbB+uGs Ph9IxWcSXHLPIqoZ43nz1gBqZ4aW245n3MSwCKoNhIGwUZ0pba0f4j/ja5EDGpnn2xbW PcetV5+mdvshpn9GuoeLJWE0cCx6lGxhc/IEnULExQ9yGPnc79Ov+Iuglk1TPnoF6gSR YiriefoE0ZRtsJev5uKqIuGxnY9/VUuG1xaQjxCgjFEXLiLc9HfyV30nGqX/vPdP1h1f sLoQ== X-Gm-Message-State: AOJu0YzGm+EKoDhG96svVEBniX997Imk98NQiHOsT0sUB8d7u2AjukQg tGj2zHJ8u4Fdqmm01wXeBB83bki2gm1aG+gJOGvNtQ== X-Google-Smtp-Source: AGHT+IG5cHClY82H0qF3KKmCwyJ734Out2F/bsDJOo9ukZYK24El35SdIJZgzp3rV1XqCgEWNwAq0ykaaXCtOVvlIrA= X-Received: by 2002:a50:a41d:0:b0:525:573c:6444 with SMTP id u29-20020a50a41d000000b00525573c6444mr3940edb.1.1695401818931; Fri, 22 Sep 2023 09:56:58 -0700 (PDT) MIME-Version: 1.0 References: <37c2b525-5c2c-d400-552c-9ccb91f4d7bf@redhat.com> <3e08d48b-7b70-cc7f-0ec1-12ad9b1a33db@redhat.com> <3408ff54-f353-0334-0d66-c808389d2f01@redhat.com> <9f967665-2cbd-f80b-404e-ac741eab1ced@redhat.com> <20230906065817.GA27879@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20230920054454.GA26860@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> In-Reply-To: From: "Zach O'Keefe" Date: Fri, 22 Sep 2023 09:56:21 -0700 Message-ID: Subject: Re: [EXTERNAL] Re: [PATCH v3] mm/thp: fix "mm: thp: kill __transhuge_page_enabled()" To: Yang Shi Cc: Saurabh Singh Sengar , Andrew Morton , David Hildenbrand , Matthew Wilcox , Saurabh Singh Sengar , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , Greg KH Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: B4E1220003 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: i7abqeun7q5iyedr5zqyp6swdiwc33f5 X-HE-Tag: 1695401820-987936 X-HE-Meta: U2FsdGVkX19s7q+LRbCA5N7xW0l28LP/viSnSPhWkp4xV/S8OcNCuXFPDtOgU35CuSVy9HXlU1nuO8HqR5jslmtxUhWc/Ct8N5pwudNZ9rwGrNRTqUQFqrpOAHsOjTBvq4R941rJFe4UKZuoJzVJ7dpVvh8KBEerwKlZep02Nwz0H/K8TeMRXAo5Cm7Yjrywa5VYGFJz40xq99/gAaLi7p1N+EvmBZFC3eDQ66KgESeoRdm0Hj41hGvFYXuV+cTvPOQbmYbeq2seHj3XB2h8UIvRCip4OzyHlq+n0IJ13rSOD7d7fMS60WkdmSDDOUdaWQyDX68v3ilosogZYOpR4S/DgeXTZnH/9lKVeB4ZQl5s/R6ZQDOphhyfh2ZjXTp+vXiHMx6+NupVq/tD4m4y1ScI3xSpn/Z7rXaeRpB00UlTS12P/goWSh/C952f+uLOEgQB2zwkl7RarumOMzcHJrgeOe2s1txCioUpDD6WF3Lr/7bD8/UhyRrnlUaGoq+3JT8UcPDF2uSm1DI98kmBs51jo6hoarSGMBHXLAMzY6/cnezHQKSMu+7z99BUduiQZ7R5zrKa39EWzne2C1kVUyRjdKy4y7LwZ+hlEaRrA3NESXAtU+yExUuHCQlruas0aTFHLODDY5XsVu5sIFWxnPVMymYXc6daUfQqotyfFFwQ/8qwH2OK+BrXzqTS4eGmkRDY+5CJSgD8fgG6V8ByAoxFk7F/KLGHeofT5MeOSYI1lHF5v8YNikPBykBHdEbVki5sN8HUIwaRJAp4HVQn1wDNLJdrcFQqA1lZeaQBLBeW4cJSLlGZCe/8QIg6gGZoULJinlyYYfvPTYnjqvHasbSPGL0Fpj0T2ayQrU9C2xm4eeRCwjWY0unQdJSY9ZVvEJWZaBghLivDZguyIRrdZCVpe+3yvTek2zfwRD0dsMw1oZKCbauTe+U6uc+R9vhRbl2PIK2aAuy9RXLA3qL /+DApaLo AxwYqb7FCI/RO+PuvM50XdpiASWGqHRwiXxq4TlcUiy214BkHdaV1mHIWOLP0rG7bmRSu5M493QSfSaXAqgtGtRMJOBJLOE9ULoHOLQApAS3Hjo4hihj+1eSYW6W3Y2ZZ8InP8mFuLmJUMIxw+6ripWrdGBTtQduS8ICBE1wS6NijT8JHQ73+KRMCt7v7yPqPv+gfjAXfKM/asokk+sKiTqFoFTDvfVNKVcfKnzvXl7dxxc61gfOYYxub1wJcDMaqsVKkPsQqmPXWIDI0g0JpR0R0juiRNoLN+E8YZRyDBFbFxzXMTOa3HhY4oZPdh84XmzO02ebIq9Lett0mXt0Igz+YNQsXhIdFbZAokM3DRVMkSIzsotGi2/TW5zXo6rhanq0Dr52hJ7Iv03IA9hqyo1K6v6k/yH/Pw5+a9kLJCn+d3vbt4pdHnBq+xtSVh2O3+U8Esc+iiwqwrX3gxh2n4bdr2v26eyU61o6jrufaUXT4iOhxG6rik8c61w== 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 Fri, Sep 22, 2023 at 9:54=E2=80=AFAM Yang Shi wrot= e: > > On Tue, Sep 19, 2023 at 10:44=E2=80=AFPM Saurabh Singh Sengar > wrote: > > > > On Tue, Sep 05, 2023 at 11:58:17PM -0700, Saurabh Singh Sengar wrote: > > > On Fri, Aug 25, 2023 at 08:09:07AM -0700, Zach O'Keefe wrote: > > > > On Fri, Aug 25, 2023 at 5:58=E2=80=AFAM David Hildenbrand wrote: > > > > > > > > > > On 25.08.23 14:49, Matthew Wilcox wrote: > > > > > > On Fri, Aug 25, 2023 at 09:59:23AM +0200, David Hildenbrand wro= te: > > > > > >> Especially, we do have bigger ->huge_fault changes coming up: > > > > > >> > > > > > >> https://lkml.kernel.org/r/20230818202335.2739663-1-willy@infra= dead.org > > > > > > > > FWIW, one of those patches updates the docs to read, > > > > > > > > "->huge_fault() is called when there is no PUD or PMD entry present= . This > > > > gives the filesystem the opportunity to install a PUD or PMD sized = page. > > > > Filesystems can also use the ->fault method to return a PMD sized p= age, > > > > so implementing this function may not be necessary. In particular, > > > > filesystems should not call filemap_fault() from ->huge_fault(). [.= .]" > > > > > > > > Which won't work (in the general case) without this patch (well, at > > > > least the ->huge_fault() check part). > > > > > > > > So, if we're advertising this is the way it works, maybe that gives= a > > > > stronger argument for addressing it sooner vs when the first in-tre= e > > > > user depends on it? > > > > > > > > > >> If the driver is not in the tree, people don't care. > > > > > >> > > > > > >> You really should try upstreaming that driver. > > > > > >> > > > > > >> > > > > > >> So this patch here adds complexity (which I don't like) in ord= er to keep an > > > > > >> OOT driver working -- possibly for a short time. I'm tempted t= o say "please > > > > > >> fix your driver to not use huge faults in that scenario, it is= no longer > > > > > >> supported". > > > > > >> > > > > > >> But I'm just about to vanish for 1.5 week into vacation :) > > > > > >> > > > > > >> @Willy, what are your thoughts? > > > > > > > > > > > > Fundamentally there was a bad assumption with the original patc= h -- > > > > > > it assumed that the only reason to support ->huge_fault was for= DAX, > > > > > > and that's not true. It's just that the only drivers in-tree w= hich > > > > > > support ->huge_fault do so in order to support DAX. > > > > > > > > > > Okay, and we are willing to continue supporting that then and it'= s > > > > > nothing we want to stop OOT drivers from doing. > > > > > > > > > > Fine with me; we should probably reflect that in the patch descri= ption. > > > > > > > > I can change these paragraphs, > > > > > > > > "During the review of the above commits, it was determined that in-= tree > > > > users weren't affected by the change; most notably, since the only = relevant > > > > user (in terms of THP) of VM_MIXEDMAP or ->huge_fault is DAX, which= is > > > > explicitly approved early in approval logic. However, there is at = least > > > > one occurrence where an out-of-tree driver that used > > > > VM_HUGEPAGE|VM_MIXEDMAP with a vm_ops->huge_fault handler, was brok= en. > > > > > > > > Remove the VM_NO_KHUGEPAGED check when not in collapse path and giv= e > > > > any ->huge_fault handler a chance to handle the fault. Note that w= e > > > > don't validate the file mode or mapping alignment, which is consist= ent > > > > with the behavior before the aforementioned commits." > > > > > > > > To read, > > > > > > > > "The above commits, however, overfit the existing in-tree use cases= , > > > > and assume that > > > > the only reason to support ->huge_fault was for DAX (which is > > > > explicitly approved early in the approval logic). > > > > This is a bad assumption to make and unnecessarily prevents general > > > > support of ->huge_fault by filesystems. Allow returning "true" if s= uch > > > > a handler exists, giving the fault path an opportunity to exercise = it. > > > > > > > > Similarly, the rationale for including the VM_NO_KHUGEPAGED check > > > > along the fault path was that it didn't alter any in-tree users, bu= t > > > > was likewise similarly unnecessarily restrictive (and reads odd). > > > > Remove the check from the fault path." > > > > > > > > > > > > > Any chance this can make it to 6.6 kernel ? > > > > ping > > I think we tend to merge this patch, but anyway it is Andrew's call. > Included Andrew in this loop. Sorry for delay -- just back from (another) OOO, >From this back/forth with David/Matthew, seems like we're OK saying, "this was a mistake", and that we can take the patch (need some form of Ack or Reviewed-by from them first, to confirm) > > Fundamentally there was a bad assumption with the original patch -- > > it assumed that the only reason to support ->huge_fault was for DAX, > > and that's not true. It's just that the only drivers in-tree which > > support ->huge_fault do so in order to support DAX. > > Okay, and we are willing to continue supporting that then and it's > nothing we want to stop OOT drivers from doing. > > Fine with me; we should probably reflect that in the patch description. But, I don't know about timing. We are in 6.6-rc2, and this hasn't been exposed in Andrew's trees yet. 6.6 is looking like it could be a LTS candidate, in which case this patch could flow backwards from -stable (which would also land in 6.1-y) .. but I don't know if that path is suitable for this. Otherwise, perhaps you could include this fix when you're ready to upstream your driver? > > > > > > > > - Saurabh