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 3DBBDE7F12C for ; Tue, 26 Sep 2023 21:39:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BD9166B0150; Tue, 26 Sep 2023 17:39:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B88FF6B0152; Tue, 26 Sep 2023 17:39:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A506F6B0153; Tue, 26 Sep 2023 17:39:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 950C36B0150 for ; Tue, 26 Sep 2023 17:39:22 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 70AF1160C1A for ; Tue, 26 Sep 2023 21:39:22 +0000 (UTC) X-FDA: 81280064964.07.B19FE6D Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by imf16.hostedemail.com (Postfix) with ESMTP id B3447180011 for ; Tue, 26 Sep 2023 21:39:19 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AZe6Y5Qg; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf16.hostedemail.com: domain of shy828301@gmail.com designates 209.85.214.170 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=1695764359; 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=y/yEtlXtVxm4kjBMlSVby9WafwRCEzcrWM1EGka6xJY=; b=0wk6eRZ9k3caD5NrH8HynxNRQ3SmpokqduX9RuP/SUejFakV1vwWCd5Ec4YPv4R6N0Cmeb 2y9P6EcI9QuRzv/GowJpjxWXWV0N02dT2lqcTpySZm/eOZmRJAVKb2soPn6gOctsBU9EWu RaiRRZt9pielW4hZL7wmDkIfXidegKE= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AZe6Y5Qg; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf16.hostedemail.com: domain of shy828301@gmail.com designates 209.85.214.170 as permitted sender) smtp.mailfrom=shy828301@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695764359; a=rsa-sha256; cv=none; b=MCQKSydX0VQVPraezpTwOowGFwI4G2/03lKwXWJpkH9Vw7+EHh4B7aGSYe5tJvTgJ/p1Or 8DcL5+U24T4OW2TaE0N8kml+kSWNvBR+wX8QWy0KMMFbQttdb0aHWlJCDr54arcMubIGV8 pyZwYDWYczMLL6qvW3VHiHExMN3Z8f4= Received: by mail-pl1-f170.google.com with SMTP id d9443c01a7336-1c451541f23so76110705ad.2 for ; Tue, 26 Sep 2023 14:39:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695764358; x=1696369158; 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=y/yEtlXtVxm4kjBMlSVby9WafwRCEzcrWM1EGka6xJY=; b=AZe6Y5QgugWyU5pbFtz4j9nDgw/z7RaWHFAVnt65uSFgVvyK1gf95i/IPf8mi+URjW eI+wG1ovfLgYi+JqHn0yrobHsG6Qluj3Wflil4oOlfaItfBG3c4fzt06ZQrMhjDwr5GN D7vJ/oPoZT+OexRVpxhmG9NVEQCTx8dmupHZTET0U32z4Byp4qAwQlVS24ASzXDokJy3 xJrU1kcFERfZWzFUURbCNomGJWc6zg8RJ37DAEc9IwXulIQldKa/aZFwyC6g9TbAsA7T N8q2c1mY8+UeVDZJtNyKmMQnQrPnfAY97AHGWxF36oCUTpqMOiH/K8CLStdqc3KQcX9J xKrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695764358; x=1696369158; 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=y/yEtlXtVxm4kjBMlSVby9WafwRCEzcrWM1EGka6xJY=; b=uxxIkhwnxwGBp7046ToD+hq87nVQYn9F+PJA3IgYKDkcAqxGVxBBEox6dViADTQ7MV M1zzMij6a4ctX4qrYY6lMV7WjSzAAguqMc2j24FDF3oh60Ige376jiHoNSzmEGZG0pCA jo6Enr4x3VA99D/L6UBEOg43isbYfBlfKX1/AHMxK5syV13lWkK5z2ecQAydt931iYTW WiAW+pwR0Cor+qh4vkp5y8siuYTog2Urd8PJqRsGsQkTw0iKBDp3TIOQzIBk8dBs5VSy J14oOtZXPTuTeUmmB1dv5Oiz7mTXNlk2wjOIWIU3ayu5C19x1hOPQ6LtMXIo6oVVacD3 kkag== X-Gm-Message-State: AOJu0YyVhD02EvoatLj2c57+Xa7+CPBDW73NUFUHhqLwY4VrUxsS0Jup 0B2SImSoM0NDPULk3wa9/cpUD6Ieu+1DGYGRAz8= X-Google-Smtp-Source: AGHT+IEYSKgDeyLNfepkexfRwh2z3vtZbnxmtLL6nlWDaFrd5K34ugg6dGSh45/zU0RvC0+WovaHBgCMMPfg8B+qWKo= X-Received: by 2002:a17:90a:7f8c:b0:274:6839:6a89 with SMTP id m12-20020a17090a7f8c00b0027468396a89mr8054307pjl.27.1695764358364; Tue, 26 Sep 2023 14:39:18 -0700 (PDT) MIME-Version: 1.0 References: <20230925200110.1979606-1-zokeefe@google.com> In-Reply-To: <20230925200110.1979606-1-zokeefe@google.com> From: Yang Shi Date: Tue, 26 Sep 2023 14:39:06 -0700 Message-ID: Subject: Re: [PATCH v4] 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 , Matthew Wilcox , David Hildenbrand Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: yygi17zeibff73ffew6r5boroanpc4eh X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: B3447180011 X-HE-Tag: 1695764359-187510 X-HE-Meta: U2FsdGVkX1+kMdWpp+yk67JnV8sAPITOU8KTPY03sF5TkwnWvqQhuX+JpouLNerso1+/DWPbOag/KhqE1GSGczPhI2THc/xXBWImRnLztw1eL7sCwRnpWpRNBlVoJaJibwfBxIXUMk8QemYwJ8b96cnJOLyGuZ77ZiafN9grP+S1ZIht8L6JWwdpzI8yYL4iVWUNfDLx+QHLm3cHGL6l+wvr+jax1bPgB2R3hw3gvQfPo+PjsFvLYm6L/iVYyYS6uswcxj1vr/alu4URZMTJEvD8c1DaOJ4YDxSqLw2REcEzOaYH4dCAJ4NBhvKtj9koZ8zV26I2CgrBM84IK5wkO947t6D1Tz8MIbASgoxJrTKYRP2iHBLc+rCOFhCrZvoenxjU24hj2qPFRBMZyOXx97h42muzRsAKwpqlYATL1FzOvW0KXWZ9uLi356Vkxfw1PCMKqtr/Lpgy2aGBHYxTce+BPpirabucEOB1r3eL+g8ZYv4ayUC26IcfrJRuJbdNEoqjLhJYP146+WBdnqcObDdwXd4zIogoXEQlLErEdsNvo+/t+C1V3PVA57nSZ8rBiplmbFWXlhg9AEJeQeDqPnjM1yPEi+K41BLntgaS9PUoKdvVOyI8E9pWqvvhlwiLsJeWa0H98lQgkRkSaMdo5f6C4rOYFU3UHKzYTImLEn8LQhSfGFu9Z1JOeYmNq50/VeCgQkXXvslZUJ1+XRkXKZtGcyBXhcGkJdeZAVsC/yKuz24YjpoKE7Ee44Dg1Og6f+rVIMyHcBHb5fWIRDQAm7Be8ZGmiZ1XlA8Xn8iyNIGhQ34+dODNT2/wOTnXAA9OimK6yHDH12IBGulePbO2/JJuoMzN9SztZsBcApgp78q3fIIrzjmvBBW4LxDjvl5zeIaYDf1XAUmz8qeTOjF3DkQnJM/9I8ivZ266ArjgBXVqLV6dGYK7248EkolujjBNuAItTyJ6gt+CGz7bzXl 1yVpE1iZ las5mLYG/tPJuRRam5wfNJhiLkkpulkaWdCB0QgSA/G6C3GeqkA5G6ZM9hUKfTOv4DNUf28CWiMBYeo1oijCvT5Ru2kS5JkjecIPgEEi7mu2SJrBydMcAmCX+yCqznaM9gq6Wc3Hsbtb6zVnh1RvjnktxnBZ3UgUMCfVHmSCnat6VY7s6OZrTI3ZYaBmxI6MNg3cqfgdNfriF3NSQNLS1D/gaSWUFoKVpfPkkiGsmT4jFyMHWHzkX7uodQezpcUyEyWQuSC5S8+Fqa6MQNznbmlK5/xZIhzFuoXnpJmYa+AN2zh4hlBwal+NcuEoJqtgWCOHgNVij0J8pl9IYIcs8fLepRwcwWOOOVGW51fNgxjsu23pvjHBXAmK3fRAncXuQH39BM2H0xY3MNSWtdkuyi2fuu9yUCDXjWpL9Ucx9A4Uk3AL4T+N/44x2f5TW+Pk6jBuqgwR9grP3VpkjdZ6ovQFoF7OulM4N8bvQFo6rqfBjnT8B7QWRlc4r2iMjTeVRuQrRGV2jmghauaKKUltrlAIDA988Pc80VAyFQyt3+BLHx08w+ITfZ1YkNk0FQWwXHC0GzCKKiWIRNQ1v1Ru2nKgwTeIWrV66YkMGsp/78Mdu20E= 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 Mon, Sep 25, 2023 at 1:01=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, this was a bad > assumption to make as it assumes the only reason to support ->huge_fault > was for DAX (which is not true in general). > > 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. Looks good to me. Reviewed-by: Yang Shi > > Fixes: 7da4e2cb8b1f ("mm: thp: kill __transhuge_page_enabled()") > Reported-by: Saurabh Singh Sengar > Signed-off-by: Zach O'Keefe > Cc: Yang Shi > Cc: Matthew Wilcox > Cc: David Hildenbrand > --- > I've updated the changelog to reflect discussions in [1] -- leaving > ack to David / Matthew on whether to take the patch. > > Changed from v3[1]: > - [akpm / David H. / M. Wilcox] Updated log to capture email disc= ussion > Changed from v2[2]: > - Fixed false negative in smaps check when !dax && ->huge_fault > Changed from v1[3]: > - [Saurabhi] Allow ->huge_fault handler to handle fault, if it ex= ists > > [1] https://lore.kernel.org/linux-mm/20230821234844.699818-1-zokeefe@goog= le.com/ > [2] https://lore.kernel.org/linux-mm/20230818211533.2523697-1-zokeefe@goo= gle.com/ > [3] https://lore.kernel.org/linux-mm/CAAa6QmQw+F=3Do6htOn=3D6ADD6mwvMO=3D= Ow_67f3ifBv3GpXx9Xg_g@mail.gmail.com/ > > --- > mm/huge_memory.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index 0f93a73115f73..797fe617e51ab 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -100,11 +100,11 @@ bool hugepage_vma_check(struct vm_area_struct *vma,= unsigned 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)) > return false; > > /* > @@ -132,12 +132,18 @@ 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)) > + if (!vma_is_anonymous(vma)) { > + /* > + * Trust that ->huge_fault() handlers know what they are = doing > + * in fault path. > + */ > + if (((in_pf || smaps)) && vma->vm_ops->huge_fault) > + return true; > + /* Only regular file is valid in collapse path */ > + if (((!in_pf || smaps)) && file_thp_enabled(vma)) > + return true; > return false; > + } > > if (vma_is_temporary_stack(vma)) > return false; > -- > 2.42.0.515.g380fc7ccd1-goog >