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 X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 14D16C433B4 for ; Wed, 28 Apr 2021 16:22:11 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 80CF7613F8 for ; Wed, 28 Apr 2021 16:22:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 80CF7613F8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B76666B00D6; Wed, 28 Apr 2021 12:22:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B276D6B00D7; Wed, 28 Apr 2021 12:22:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 99FEE6B00D8; Wed, 28 Apr 2021 12:22:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0005.hostedemail.com [216.40.44.5]) by kanga.kvack.org (Postfix) with ESMTP id 7D0666B00D6 for ; Wed, 28 Apr 2021 12:22:09 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 3730112F4 for ; Wed, 28 Apr 2021 16:22:09 +0000 (UTC) X-FDA: 78082292778.27.6ECD489 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) by imf18.hostedemail.com (Postfix) with ESMTP id CF5352000254 for ; Wed, 28 Apr 2021 16:22:10 +0000 (UTC) Received: by mail-ej1-f46.google.com with SMTP id a4so2152325ejk.1 for ; Wed, 28 Apr 2021 09:22:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=oMB6lxmPnvhCab0TpujzWdPU0Z4pjoErZwLGf29ahVs=; b=rbWVvBwOF0IKkWL6UqJvyx5PA9puqWJNnasIOJRVGqj9NfFdC4XB1IShjwmEo6eAe5 9LMU6+YxYMCBCGkHK5kHKch+fScAhE7sPwM+fH2BGttU53f7QfvcN2AQ2Aqx2VVauBlA vlZx0I8cmuiofoTKBs9Xh8XU0pMbCW0QKHz3GLmBfGdRmF92H7oQxy7pnkzDwYVDer22 CAK/uEOoL6iHWOjnuHw/bsyxoM+dhUu+Kq8+QIT0ZlShqyC62Fr71zZm2TpKhR0rhxzN cfgTvHLLkXpiHiTlG3LHrcLvtPNN/UoESKoO5hFh/9aA8Yai525nr6nPfoOFKONxPm+2 9UyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=oMB6lxmPnvhCab0TpujzWdPU0Z4pjoErZwLGf29ahVs=; b=TLu4rSx1RZfhsGnME6Qb7NRYlRaCjx0xbXlCZcKjpbCo1SsVk8nAI6Uu7r9AM75SMf REMKg3vhxjgLxJQdoKhHtADflhdlXPlbJCxK92HyCB4WQRWEGVr+Quj356sAop4hDOwZ vjdR0/7ui7VFth+zx1r85MzwK3ZNVtdohLzGJBEGhceveEyXcsWT+hEJxuJ+wrTBqDWJ sShJ3XOXxIXaAW8Pg62n55cppvfgcYn1YZfHM3l+lBTzraNJJZ5d9TSBf/SBAzj6zkuS WGgpjyPLMADgQ9LPnRFFgbWb0yVgmP6zjhSx+5mxT5FPJiJEux1QZnjPfLYdvGp/SZxv Ah7w== X-Gm-Message-State: AOAM533xoVlGDHODgKnPf4/YaB7E70B2HdGcnz5NhhJMoLLPEN7YRCmi T/0kvNaruGumqJI/aUk3i+ggc0l4Fcc/InVmp54= X-Google-Smtp-Source: ABdhPJwRBzNaPHfGJyPs1X6LjTtSk+HM8vD2TZkmDC2eYJjlc920w3dQIrDcjNcECOkgxHQ71daNirKgDsMP5XH0Ulc= X-Received: by 2002:a17:906:a51:: with SMTP id x17mr30322064ejf.25.1619626927582; Wed, 28 Apr 2021 09:22:07 -0700 (PDT) MIME-Version: 1.0 References: <20210427133214.2270207-1-linmiaohe@huawei.com> <20210427133214.2270207-4-linmiaohe@huawei.com> <1fa95721-2ae0-af5f-b2e4-cdb430ebc263@huawei.com> In-Reply-To: <1fa95721-2ae0-af5f-b2e4-cdb430ebc263@huawei.com> From: Yang Shi Date: Wed, 28 Apr 2021 09:21:55 -0700 Message-ID: Subject: Re: [PATCH 3/5] mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled() To: Miaohe Lin Cc: Andrew Morton , Zi Yan , william.kucharski@oracle.com, Matthew Wilcox , Yang Shi , aneesh.kumar@linux.ibm.com, Ralph Campbell , Song Liu , "Kirill A. Shutemov" , Rik van Riel , Johannes Weiner , Minchan Kim , Linux Kernel Mailing List , Linux MM Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: cng97esbaordp95bncizfzmc77ijnqyz X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: CF5352000254 Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf18; identity=mailfrom; envelope-from=""; helo=mail-ej1-f46.google.com; client-ip=209.85.218.46 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1619626930-813695 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 Tue, Apr 27, 2021 at 7:06 PM Miaohe Lin wrote: > > On 2021/4/28 5:03, Yang Shi wrote: > > On Tue, Apr 27, 2021 at 6:32 AM Miaohe Lin wrote: > >> > >> Since commit 99cb0dbd47a1 ("mm,thp: add read-only THP support for > >> (non-shmem) FS"), read-only THP file mapping is supported. But it > >> forgot to add checking for it in transparent_hugepage_enabled(). > >> > >> Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS") > >> Signed-off-by: Miaohe Lin > >> --- > >> mm/huge_memory.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c > >> index 76ca1eb2a223..aa22a0ae9894 100644 > >> --- a/mm/huge_memory.c > >> +++ b/mm/huge_memory.c > >> @@ -74,6 +74,9 @@ bool transparent_hugepage_enabled(struct vm_area_struct *vma) > >> return __transparent_hugepage_enabled(vma); > >> if (vma_is_shmem(vma)) > >> return shmem_huge_enabled(vma); > >> + if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && vma->vm_file && > >> + (vma->vm_flags & VM_DENYWRITE)) > >> + return true; > > > > Many thanks for your quick respond and Reviewed-by tag! > > > I don't think this change is correct. This function is used to > > indicate if allocating THP is eligible for the VMAs or not showed by > > smap. And currently readonly FS THP is collapsed by khugepaged only. > > > > So, you need check if the vma is suitable for khugepaged. Take a look > > at what hugepage_vma_check() does. > > > > And, the new patch > > (https://lore.kernel.org/linux-mm/20210406000930.3455850-1-cfijalkovich@google.com/) > > relax the constraints for readonly FS THP, it might be already in -mm > > tree, so you need adopt the new condition as well. > > > > Many thanks for your comment. I referred to what hugepage_vma_check() does about > Read-only file mappings when I came up this patch. But it seems I am miss something. Yes, you need do the below check for readonly FS THP too: if ((vm_flags & VM_NOHUGEPAGE) || test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags)) return false; This check is done separately for anonymous and shmem. It seems not perfect to do it three times in a row. So I'd suggest extracting the check into a common helper then call it at the top of transparent_hugepage_enabled() . The helper also could replace the same check in __transparent_hugepage_enabled() and shmem_huge_enabled(). > Take the new patch into account, the check for READ_ONLY_THP now should be: > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index 76ca1eb2a223..a46a558233b4 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -74,6 +74,10 @@ bool transparent_hugepage_enabled(struct vm_area_struct *vma) > return __transparent_hugepage_enabled(vma); > if (vma_is_shmem(vma)) > return shmem_huge_enabled(vma); > + if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && vma->vm_file && > + !inode_is_open_for_write(vma->vm_file->f_inode) && > + (vma->vm_flags & VM_EXEC)) > + return true; > > return false; > } > > Am I miss something about checking for READ_ONLY_THP case? Or READ_ONLY_THP case is ok > but other case is missed? Could you please explain this more detailed for me? > > Many thanks! > > >> > >> return false; > >> } > > > >> -- > >> 2.23.0 > >> > >> > > > > . > > >