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 29A23EB64DD for ; Mon, 14 Aug 2023 18:48:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B1320900006; Mon, 14 Aug 2023 14:48:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AC3C0900003; Mon, 14 Aug 2023 14:48:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 98B26900006; Mon, 14 Aug 2023 14:48:31 -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 8951C900003 for ; Mon, 14 Aug 2023 14:48:31 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 559A380311 for ; Mon, 14 Aug 2023 18:48:31 +0000 (UTC) X-FDA: 81123596022.16.347BEC6 Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) by imf21.hostedemail.com (Postfix) with ESMTP id 79A691C000A for ; Mon, 14 Aug 2023 18:48:29 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b="JdfU/5ru"; spf=pass (imf21.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.50 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=1692038909; 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=JyHWTzG4ijgJg3zlJfOxdGz41fwifX3dI+PZ0fOTOHA=; b=2kYA5l2WrGsCXevVE88sUe7aNh05PF80H6qGewulTwH/isp7dh6GrXXMfCidAC/bNqmUbS iGYrni0PU7SlOwk86pFB7BrphaBcGHiDi1voGSxD2rBPPUvORvclauPU2zRrBqh62Vpcl9 w6SRGJt412sppm5/mFrtbOkmSamY6BM= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692038909; a=rsa-sha256; cv=none; b=3/W43f7a0Ws3CMSxh/okpSwJ5xl3qL1eb5heFadYl9f2UCJMg3v9ffitlIA5n+KznU2cWP cfH3FkEMvR2agAN0liSo4GUGBthhEB6f6OZfMopW0OcNnkUvEg0MNgHV/bii3Q+13zl3L8 2x5V5M7TvrOIMfVHPJDjHv+TODJE8mk= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b="JdfU/5ru"; spf=pass (imf21.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=zokeefe@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-521e046f6c7so2351a12.1 for ; Mon, 14 Aug 2023 11:48:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1692038908; x=1692643708; 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=JyHWTzG4ijgJg3zlJfOxdGz41fwifX3dI+PZ0fOTOHA=; b=JdfU/5ruki1e7gDB/9c4eSChzNb303LI9YXp0mpWVposcgzf8PxhYBCSAzeTgpP1eI HvX17Rh/hxtGWwpLdp5zDg4vTq8SXuhJch908IoPSevK2Xe3jisJfSeEghxnosCospoM xJ5320wZXMIHROtoQBOcyuIxddXpOHDBjykqsm+PnAQ1/4H7dcFUL/GFnFkOxHG4MJlg WDaXMulNdn1Ku5n6+9PuPwFJ+c41JoTyA+XX0Xhx4JWait3HJQQTBLuq4ofakP/qR+ZV TfYhjrhTEthCEvPtrhu7iekhM3SoWEwBN7BzRzqQIsRM+fUr8hYvX9MFNPW6qKco2Ce3 KXvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692038908; x=1692643708; 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=JyHWTzG4ijgJg3zlJfOxdGz41fwifX3dI+PZ0fOTOHA=; b=XGji14Ooh0OWv9nSg0+363jyU0B7E2YRDIAwsvEVhYWgJPOvGE4TLSx0rCzegmDxyA PKgckGHUn1ip1hiPV9OIqgvD7J+xnmMOb2ve9Sx0npIyDXpKaAgmBIfVLREQn9KEld/8 /rvgzQsapex8IWVga55XG7JqN/li93mdgWO9OG7NYz1mDeGxtg66w5thn3sXl+sEcODT DY7pzBGhBym3tlCa11hjiWhOdCMgvcAR/IvJ+W++RasBRsF9Am0+b3+rc5ap9Too2Su1 UTBvW8UsBAmde5sk3oxANgjdPVgIdlR7d534lTE6+WzCpypQREeSR8uzyU4NGJIlLIJN wDqg== X-Gm-Message-State: AOJu0Yw+pJTa8elMbMuaSk3HpL7B0FH1Y02OXiZ1wBJnWtFQjNw0iDrR oKka1ovJUHW8Ol9ZdHs0UVp2tEN0P+TBCMYm3g6S2g== X-Google-Smtp-Source: AGHT+IEcpK4nmeQ2r+LKdimObrzoGBi182a6AUbULOFCc73Z+MR7QTMJtr8LUewd2DtlYEUprqKzfXt2yGhNTGmumNo= X-Received: by 2002:a50:c30e:0:b0:522:cc9c:f5a4 with SMTP id a14-20020a50c30e000000b00522cc9cf5a4mr296056edb.4.1692038907622; Mon, 14 Aug 2023 11:48:27 -0700 (PDT) MIME-Version: 1.0 References: <20230812210053.2325091-1-zokeefe@google.com> In-Reply-To: From: "Zach O'Keefe" Date: Mon, 14 Aug 2023 11:47:50 -0700 Message-ID: Subject: Re: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill __transhuge_page_enabled()" To: Saurabh Singh Sengar , Matthew Wilcox , Dan Williams Cc: "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-Queue-Id: 79A691C000A X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: xdrfxbg5n3uik4xqkoddgefg6rb793eg X-HE-Tag: 1692038909-118327 X-HE-Meta: U2FsdGVkX1/sNvTCqG36aEtIeKu6agoTPwhEOaKRu4hMgFoqaZHZfI55KDAqvR05mA/Pv5xeBHYr4v0vQlGGHLXr0JXb12sXYveCQunaLjqAZQqS7rgFeSX03f3K6EWJxOEchhHtNcSg5CVq77nR59lnKqJYCNHtirfDDc6d0Zrbwd2ZwtUihzczDN0EPH4BwTrgcvcJrze/wVAGqoQsaehmTle0SUkjOH4xah/YmBV2O85FTfft3A6D0nni4L/D4AeF02Id/3cM+2Jh1NjzIXbK0UCGTJPprlRZ45YDo5kvsSog3/z03+mU0looXtx/Fq4Zx18c2McbhspHoKMt0YqOOdT5O21IS/Txg5i6qLbYGehDTRVSV2t49jXfUZ9rjpbQqEIYmZrWr8KJfZPznyySQTLluxSgrWremQjhgLTcwbs9WQ83u9fUDOSspWbryHUYpB3RHZoAC3hGjubwC7agtXvOgTnXKPosH9sqzcE4sI2nVd0g4ZHT72d5fnh6HXY7aDXT2sz3uinDOUnp/orAiLTGlzk6udezoKRg6PX/P+74IcCJFtAKFHjEWDnlk2APppD941It1ckEDaVvoDHhkLVnR3hBY0zMtV2hFpAmihP0UWW8bY7UH1vm+aOdpyA4vQ1+IIs9xG6zF/bu6Ru8KKbYtFN2eb8XtFfZRlr+FJBGkk3Dlqj1GLz0xLICMtT0XnGMZvjvhDnsb3t2Y7LryLhgjoF3+S3ESmvZS6LiT/Mq+NzeVn+CG6AN1m73gsoc1A38e1Kkqo4fZ3R3olkzqwXC6uxCc3I3yHmYQMtpgb8pwdFTkMnvuZA0SSd+ItVfbMN81s8/an+q4E6KDkz18cTt0Ba5smX9fiopMkAiq6jaO6+6jbGVjAj4Yf1AxEWVRfMlBC6goEp8Vt2gxSse2rKzh844T+Eh48txX+/pOu7amy285AUzGYyBZkHWCDehJh2+RmzhNnwbnjj w2PrltzN E+LsHwGPj4EtxV7Sw19YF7iIjsCV+O61q7pehW9Rk4KSCwdoHr+rUuNposnRa92fYHeULgqhyvWafc6KLTKn7QGh8i7pKfdRSAJgI85MqhGI81CMBZ4ub4DfANBa+Npu1VpUvhHZR8SBZ7J6gg/C44EjXe/q/o6lbaOUzoJPB1qZ8iblgSqZ2N5WRXjfGNJGUD9Lgz2yM1Bcs8k3+zGMJjnPaDj1XHmua6MItRkQkk+aU1uHr+QhNpbjSx3oWVstnE3uRw3vYgJgP/3zLLo/haObJcRE4TaAwZC55/RRz4XNbh0OVLkUQcdZ64sXXgq+v6pIvOsjgKI30tZVr2o7wGOdxRW3BlA3FyUP4nwF7WWyowmmxr4aoR8XzJSTE84yjFhpA3AEFID/ZYu03ldGOJl4u+qCZiLVblsCchk8xtyQTsKYWTy4bR+zR2IcrqgSat1FzGvM6NErOXYLlGb+TRBXCNHYOj/FIq4VUvGxq0BvMTdU6pGko+i/EwhUMirEU3AWJc+kVlxeX6HpBuAyckIohEBHvIY3nX+NtXS5Tta5flmHs7UiFt2+lYv9YdIooampCNepeE6IQwJs= 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 Sat, Aug 12, 2023 at 11:19=E2=80=AFPM Saurabh Singh Sengar wrote: > > > > > -----Original Message----- > > From: Zach O'Keefe > > Sent: Sunday, August 13, 2023 2:31 AM > > To: linux-mm@kvack.org; Yang Shi > > Cc: linux-kernel@vger.kernel.org; Zach O'Keefe ; > > Saurabh Singh Sengar > > Subject: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill > > __transhuge_page_enabled()" > > > > [You don't often get email from zokeefe@google.com. Learn why this is > > important at https://aka.ms/LearnAboutSenderIdentification ] > > > > The 6.0 commits: > > > > commit 9fec51689ff6 ("mm: thp: kill transparent_hugepage_active()") com= mit > > 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". > > > > During the process, the check on VM_NO_KHUGEPAGED from the > > khugepaged path was accidentally added to fault and smaps paths. Certa= inly > > the previous behavior for fault should be restored, and since smaps sho= uld > > report the union of THP eligibility for fault and khugepaged, also opt = smaps > > out of this constraint. > > > > Fixes: 7da4e2cb8b1f ("mm: thp: kill __transhuge_page_enabled()") > > Reported-by: Saurabh Singh Sengar > > Signed-off-by: Zach O'Keefe > > Cc: Yang Shi > > --- > > mm/huge_memory.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c index > > eb3678360b97..e098c26d5e2e 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -96,11 +96,11 @@ bool hugepage_vma_check(struct vm_area_struct > > *vma, unsigned long vm_flags, > > return in_pf; > > > > /* > > - * Special VMA and hugetlb VMA. > > + * khugepaged check for 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; > > > > /* > > -- > > 2.41.0.694.ge786442a9b-goog > > Thanks for the patch, I realized with the commit 9fec51689ff60, > !vma_is_anonymous restriction is also introduced. To make fault path > work same as before we need relaxation for this check as well. Can we > add the below as will in this patch: > > - if (!vma_is_anonymous(vma)) > + if (!is_pf && !vma_is_anonymous(vma)) > return false; Hey Saurabh, Thanks for pointing this out, and sorry for the mixup. I'll try looping in some folks from DAX and fs worlds to be sure, since my knowledge doesn't extend far into those realms. I was under the understanding that CONFIG_READ_ONLY_THP_FOR_FS was supposed to keep the filesystem blissfully unaware of hugepages; IOW, that assembling file-backed hugepages was supposed to be a pagecache-only thing .. or be DAX. The early check: if (vma_is_dax(vma)) return in_pf; Should handle the DAX case. IIUC, the check, lower down: if (!in_pf && file_thp_enabled(vma)) return true; Was supposed to be the last check for eligible file-backed memory, and here it's clear that we don't support faulting-in hugepages over file-backed memory. Looking at current users of struct vm_operations_struct->huge_fault, I see: drivers/dax/device.c : dev_dax_huge_fault fs/ext4/file.c : ext4_dax_huge_fault fs/xfs/xfs_file.c : xfs_filemap_huge_fault fs/erofs/data.c : erofs_dax_huge_fault fs/fuse/dax.c: fuse_dax_huge_fault All of which *look* like they operate on DAX-backed memory (I checked the xfs handler, it does so as well) -- so they should have been whitelisted by the vma_is_dax() check. All this to say, the kernel doesn't _currently_ support faulting-in hugepages over non-DAX file-backed memory. However, it seems we don't give that ->huge_fault handler a fair shake. Saurabh, does your use case fall outside this? Willy -- I'm not up-to-date on what is happening on the THP-fs front. Should we be checking for a ->huge_fault handler here? Thanks, Zach > - Saurabh >