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 75E8BEE49A5 for ; Mon, 21 Aug 2023 22:53:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D2081280002; Mon, 21 Aug 2023 18:53:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CCF9E94000B; Mon, 21 Aug 2023 18:53:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B7064280002; Mon, 21 Aug 2023 18:53:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id A162494000B for ; Mon, 21 Aug 2023 18:53:26 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 7EC0F80145 for ; Mon, 21 Aug 2023 22:53:26 +0000 (UTC) X-FDA: 81149614812.26.D1E55A2 Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by imf11.hostedemail.com (Postfix) with ESMTP id ABA5D40010 for ; Mon, 21 Aug 2023 22:53:24 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=jPWb0Gs+; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf11.hostedemail.com: domain of shy828301@gmail.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=shy828301@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692658404; 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=mCqfnO9Be/Ab7vuptHUo6uWeUi2p2xjbtI/EZiHxUng=; b=BlOqtkAQ24DkIQLaM2vPwIkD66cHRxJbaKv2nsIso5MhJ8uy21sHnFPfkdXQzoMUdqLFKF Vi51M7JPMT2qLfZLtG6/u+37oNF++Qbfnp7vwyVUD0IeWaqO5l03OMIBZtDAJb7C/ZXIUj BaEielJ7WliX2ntUtpArgmdvGxypBQ0= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=jPWb0Gs+; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf11.hostedemail.com: domain of shy828301@gmail.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=shy828301@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692658404; a=rsa-sha256; cv=none; b=lAFIfsUYnAS3wvp56lUOtBK0OwEl9XhaO6XN3KWPmLNPWm7T9G5NDx/f/CugiATZN4GAkd h+2bBkM8A31mrOzmIJSWXjSpQ6Q9xvxkz60jPjTjZ9DYmKpUJVzeP5MwYxQuxvJMNG7E/l 2+GIcW4aa0Z6oSHzA7OFOOy65UMuhGI= Received: by mail-pj1-f50.google.com with SMTP id 98e67ed59e1d1-26934bc3059so3347892a91.1 for ; Mon, 21 Aug 2023 15:53:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1692658403; x=1693263203; 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=mCqfnO9Be/Ab7vuptHUo6uWeUi2p2xjbtI/EZiHxUng=; b=jPWb0Gs+6fFujjWIELjliELM3jzG6DXdK7fKUc7wtJ4kO60unkDVztk5aUhZiusU1F diNVy1zNpO7XXlh2pDKZ9IHjCbLUUZtR3WghjVjKsJTSkPtByoUBXB/kcrD44kJpn75T nmrtOmrAGz4dmDeT+rCH67xTqXYS0GPoRWlC4X5WCHTrzKuqPAZl+/fHMP3z8FMWrHs2 QmDH5IRFp0kWYUlNpX5n9NzaElwNuAtqioMq34yXmOg8fwJIo9qG6ygvJsgrFPJ+tPwA lv5e8lsAkHAZTf7E2U8HI1yhjzGn3ZneGYrnNqzIAP7HgPJlf/LH4oQzHYBGqsPjsS7R /MQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692658403; x=1693263203; 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=mCqfnO9Be/Ab7vuptHUo6uWeUi2p2xjbtI/EZiHxUng=; b=fA7TCD2SLaONyc5cp0FCaytKlFlGbTiOEzr+8OrQrLuyjy1552mxA6pl3+45XrWQaR z+c0FT2fz3m6o1q6oEtDDcDIQNJ/Ns4F6T1dbkK4BgyVbQ47QMVJ70GQ67i/r3+7XNGL xwyXw8pt2DUG/7dlBqvOojDQ13eVwBXJaxuUHgpRHYrl8pNJGQWF7xDCwlr0IaStog3U YJ45Vzjfptmp6NQvflX359m+EhpD3v7OsHVRXnM3mCtOxd3RO5ZePSlZVrXv62mbQNZV cXMCoAoSWuWoxcaMBLOZ8J4UuMVEBdYdjuaZz4kGs5Zz2Awc+qJyT5ekKaz9NeNFq9lA PVpA== X-Gm-Message-State: AOJu0YxuqeMsZq7L3QdCHb+Im9dq4NKg2QFl6T5REdGLogAXpIWYIi9Q +1iJAF1ZVeZOgybC9QWpmAUrYGNOEMioTxGQ9HQ= X-Google-Smtp-Source: AGHT+IHzIOsbe21+zcESDbwwQACz/iqWoMYJDXNxMd8iJ4rglF5vLBDSW47hCpc2svBr/ec1hFrM3HsXSyqqDHj2w0s= X-Received: by 2002:a17:90a:df0d:b0:26d:609a:74cd with SMTP id gp13-20020a17090adf0d00b0026d609a74cdmr10262519pjb.24.1692658403364; Mon, 21 Aug 2023 15:53:23 -0700 (PDT) MIME-Version: 1.0 References: <20230818211533.2523697-1-zokeefe@google.com> In-Reply-To: <20230818211533.2523697-1-zokeefe@google.com> From: Yang Shi Date: Mon, 21 Aug 2023 15:53:11 -0700 Message-ID: Subject: Re: [PATCH v2 1/2] mm/thp: fix "mm: thp: kill __transhuge_page_enabled()" To: "Zach O'Keefe" Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Saurabh Singh Sengar Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: ABA5D40010 X-Stat-Signature: 5gkzyue8q7kxkpue5b6ppzxbpeytf6c8 X-HE-Tag: 1692658404-718334 X-HE-Meta: U2FsdGVkX1/3Xj8tHtYRxAZ5+2GHPy5VkYqoblDG7IQqQAbB8RFlSw9Usj3odfoaCBoksrhT3NN8lz7Ehae+B7YXQ/nZNwbara/vh+ZxH9i8F2k3wK2pcDqyh2VCJmmEt675lwYkpcXHQCcpnIZCd/UltbXtxJ7hErcvJQ7YBrF+y63eU6ViqiydXmoT0ZSI9ygsNMzdFoGVbJ34bkpFwn8tFusB9X98tv9Bq8+1FEHSqhxPWvwy2jM4hLhO9kCTraPIujkPzZw2V24qFP5t5DMgnJLMbqRlrx1Cg3sJjnNd5rLN//oW7npB4ydF8FPW8mMYc3JZyiKcTbbVuq9ZR7WeEHQi7lCTQhIDOmVlhTWl4+8UuP0EqqkzpvIy7BuEo9HYwcTipOksAZMiDFH8513rxBpgitQTmQNOuOlJq12KE62dme9iVp9iDT3EiewVjfGGp4SWbSYRYF6h7WLsXlhT/VYnps53yOkK3TyhgRdA00Dt8tjpnZyHzHlf0c7UVshM6S94EYwxk5QXSTwQ7TbKKI91gtOJ8W/nrECFL/KHtLCyzATGT+bsyKtgXk6b+4qDDBT1ChXhNRimpdlA+9goHASKFBYxQqhc2+7A3pzXjgwwpvd8fNvBL8Mt7E7u2/G3LvkFx2JPc5MWlmh7fuS1FV2En+tu5BsBgIKSdSXNXtT9AhUrwhl8fs166aPPRQizkcgUYLNWrHjQA4KFWt3XEDSKQDpIy+4l5LwP/IgRdiMfYCN9peSLDGOr5kWYxZeYjdaxqu2/xVrq+T0P8NMyDzrLbzHFFGFG9F2wgn9RT+vBbxNr+g9asbmauUZdMm7s2NMCqE5P94QQ8ziSAQ+1owiN/TbTJGE4ABCmOhYvHkZPM0ehy3RaBbSDyHMCNywytA6rKowsuQ8Te512GUGDvKjhtzAY2doh3D7E2U64rklohtmbVwATaB3mEtBCp9fy/h12AaYImqVkCFU imn0szuf MTOF/LU21uFktNILiEZhXCOfoHpYa/OS0sCAkDrDwSH+duE1QPi93FxTaWf0LcuSFBcZE9XfTYQqQfY0RfW07HXgXGo6D1PoTjiwaC4uVGGfqqRDsvRWYzrz7uigCIi2bY3von/7MznqUpMcCGm56jVlD86j+FWBIw5zqMzm+5+nfloVzttzHDtwD4HdwtW0dmEQO3P33i/Z1wW9Q3gIErhekkFKssQenosEWyrPD/sa5H/6KO/T3JwMyYBTaDxgnt/mHMruW44GU0C8aXC7jPQYRsHHuznBeg3rhwGkdGGXvFrmWlOAI0yVqtSjPWTN6AYD55xpT3VZFJayqcGd1YVXXhLTqJYeC4BApmlVAwRlUiYwghmiwm3BfD8YwsQSoBJs1Pf8GyVa6N/jEyqmPY1QRbGQwGQ9w3eSQMoacTQtgAY0tGfpD42AXclx5hA03mNJ1qQjAnTASMRWQDe7JElXXWXJJYlR1kCQrLvQwBfch0/2ZrzNQONtEjFckj+EXKnBpnYao7SlGZG0BXYWDCKlaZA== 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, Aug 18, 2023 at 2:15=E2=80=AFPM Zach O'Keefe w= rote: > > The 6.0 commits: > > commit 9fec51689ff6 ("mm: thp: kill transparent_hugepage_active()") > commit 7da4e2cb8b1f ("mm: thp: kill __transhuge_page_enabled()") > > merged "can we have THPs in this VMA?" logic that was previously done > separately by fault-path, khugepaged, and smaps "THPeligible" checks. > > During the process, the semantics of the fault path check changed in two > ways: > > 1) A VM_NO_KHUGEPAGED check was introduced (also added to smaps path). > 2) We no longer checked if non-anonymous memory had a vm_ops->huge_fault > handler that could satisfy the fault. Previously, this check had been > done in create_huge_pud() and create_huge_pmd() routines, but after > the changes, we never reach those routines. > > 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 releva= nt > 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 broken. > > Remove the VM_NO_KHUGEPAGED check when not in collapse path and give > any ->huge_fault handler a chance to handle the fault. Note that we > don't validate the file mode or mapping alignment, which is consistent > with the behavior before the aforementioned commits. > > Fixes: 7da4e2cb8b1f ("mm: thp: kill __transhuge_page_enabled()") > Reported-by: Saurabh Singh Sengar > Signed-off-by: Zach O'Keefe > Cc: Yang Shi > --- > Changed from v1[1]: > - [Saurabhi] Allow ->huge_fault handler to handle fault, if it ex= ists > > [1] https://lore.kernel.org/linux-mm/CAAa6QmQw+F=3Do6htOn=3D6ADD6mwvMO=3D= Ow_67f3ifBv3GpXx9Xg_g@mail.gmail.com/ Thanks, Zach. The patch looks correct to me. You can add Reviewed-by:Yang Shi . A further comment below... > > --- > mm/huge_memory.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index eb3678360b97..cd379b2c077b 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -96,11 +96,11 @@ bool hugepage_vma_check(struct vm_area_struct *vma, u= nsigned long vm_flags, > return in_pf; > > /* > - * Special VMA and hugetlb VMA. > + * khugepaged special VMA and hugetlb VMA. > * Must be checked after dax since some dax mappings may have > * VM_MIXEDMAP set. > */ > - if (vm_flags & VM_NO_KHUGEPAGED) > + if (!in_pf && !smaps && (vm_flags & VM_NO_KHUGEPAGED)) I'm wondering whether we shall remove VM_MIXEDMAP from VM_NO_KHUGEPAGED or not if that kind VMAs are huge page applicable for some usecases. The downside may be some CPU time waste on the VM_MIXEDMAP area which has PFN instead of struct page, but it should be ok. Anything else did I miss? Just back from a long vacation, my brain is still not running in full speed yet. > return false; > > /* > @@ -128,12 +128,15 @@ bool hugepage_vma_check(struct vm_area_struct *vma,= unsigned long vm_flags, > !hugepage_flags_always()))) > return false; > > - /* Only regular file is valid */ > - if (!in_pf && file_thp_enabled(vma)) > - return true; > - > if (!vma_is_anonymous(vma)) > - return false; > + return in_pf ? > + /* > + * Trust that ->huge_fault() handlers know > + * what they are doing in fault path. > + */ > + !!vma->vm_ops->huge_fault : > + /* Only regular file is valid in collapse path */ > + file_thp_enabled(vma); > > if (vma_is_temporary_stack(vma)) > return false; > -- > 2.42.0.rc1.204.g551eb34607-goog >