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 21651C2BD09 for ; Mon, 1 Jul 2024 14:51:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 096F46B009A; Mon, 1 Jul 2024 10:51:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 01F7A6B009B; Mon, 1 Jul 2024 10:51:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DDB1A6B009C; Mon, 1 Jul 2024 10:51:34 -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 BB9146B009A for ; Mon, 1 Jul 2024 10:51:34 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 4689312124B for ; Mon, 1 Jul 2024 14:51:34 +0000 (UTC) X-FDA: 82291472508.21.22E42EA Received: from out0-222.mail.aliyun.com (out0-222.mail.aliyun.com [140.205.0.222]) by imf07.hostedemail.com (Postfix) with ESMTP id 17DCA4000E for ; Mon, 1 Jul 2024 14:51:29 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=antgroup.com header.s=default header.b=htRDH1iT; spf=pass (imf07.hostedemail.com: domain of libang.li@antgroup.com designates 140.205.0.222 as permitted sender) smtp.mailfrom=libang.li@antgroup.com; dmarc=pass (policy=quarantine) header.from=antgroup.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719845475; 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=7fXQtGE1vB/XPlQsFz75kPV4wKlykVtl82FG6M2N7A0=; b=u0swHqEAE24PxfdAdxshwrBp0xgYxoElNTL0w12+0aF5KsuCRb6l+0y8ZoMBDMUn5FrwVK gROWiraj6kIyS+2HhDKW99JYPqTeXrnZbWzoIInJleaZfks4v/9teTuL0IJSzwm8ZSdyi4 ZXcIcWduomL21+SC215NmDdPnXnOsF0= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=antgroup.com header.s=default header.b=htRDH1iT; spf=pass (imf07.hostedemail.com: domain of libang.li@antgroup.com designates 140.205.0.222 as permitted sender) smtp.mailfrom=libang.li@antgroup.com; dmarc=pass (policy=quarantine) header.from=antgroup.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719845475; a=rsa-sha256; cv=none; b=KO1v3v1IpwA4F0Urzg1M1AbFpCcEEcg7mS0ZLZbDt1r727oBmKgV6W6W+ceQM9Rpr+4mhI GeS73v/GfQZ8tMb5wTqGOYR0jHllO1cl4ZsHITgszZEywguqlE6aZxVIrR8Xe5Kmjde6In 95eQghX+giX+M7NcCbGntrbBQhHIp94= DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=antgroup.com; s=default; t=1719845484; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=7fXQtGE1vB/XPlQsFz75kPV4wKlykVtl82FG6M2N7A0=; b=htRDH1iTjFjcRFDlQ7H2nc2Wzn5N3iPqW+k+KltRWTfTCboTz/qDSU4q8+pM544FSFrLbawSJB2iS9+ZGWf6ZD1v7QJO4Gh+onlKtTUek2MrqhlBy8Bg3mKf/kNlf+CTNcGuV7N5hWVFQPeOUTKA+/qn7OD6hSlwwJKZCGI1a1g= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R121e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam033070043001;MF=libang.li@antgroup.com;NM=1;PH=DS;RN=9;SR=0;TI=SMTPD_---.YENyL8G_1719845482; Received: from 30.13.185.168(mailfrom:libang.li@antgroup.com fp:SMTPD_---.YENyL8G_1719845482) by smtp.aliyun-inc.com; Mon, 01 Jul 2024 22:51:23 +0800 Message-ID: Date: Mon, 01 Jul 2024 22:51:20 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] support "THPeligible" semantics for mTHP with anonymous shmem To: Baolin Wang , Ryan Roberts , hughd@google.com, akpm@linux-foundation.org Cc: david@redhat.com, wangkefeng.wang@huawei.com, ziy@nvidia.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20240628104926.34209-1-libang.li@antgroup.com> <4b38db15-0716-4ffb-a38b-bd6250eb93da@arm.com> <4d54880e-03f4-460a-94b9-e21b8ad13119@linux.alibaba.com> <2910ddc3-b05f-4394-8288-6e3c321fffee@linux.alibaba.com> Content-Language: en-US From: "Bang Li" In-Reply-To: <2910ddc3-b05f-4394-8288-6e3c321fffee@linux.alibaba.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: w1wfn5a8kocfp8rjwbd5nbjs6gwfzgtb X-Rspam-User: X-Rspamd-Queue-Id: 17DCA4000E X-Rspamd-Server: rspam02 X-HE-Tag: 1719845489-330977 X-HE-Meta: U2FsdGVkX1+B+tTzo7hNgLySbrNAil6+GhMEVGgk89Tp96B4BoHjgN8K2GkNXiYYKhyuYymydNAMkxzOPzrZK3j3IKMAXfYGuIHWBrOjuqF4Mcc2Sz35stoKtFrzGk05OIgUbwiSzlyWtD02j/5Kdc62GgV/iFjHBLF0qe9WXn0VVm8+7LJ+KFSGmo6nNJLS3JlQZ6VSlwMaB6iY8UL6OEPv2ZHfCDOTjd1atISvK/8tMYKHAz2jLbTl56PYclGxcbWdY4LbDt0r2uFWEJJEw/DPCMZVQmKVmxSR0kdbXWCVp+mT/IaQ8ouyBEVhnnx9eZanqOWHarohuOlDEuK4D+QJqygIKq5eDMILctfy9z8ayyy0huTLPsm6yY69//6sjxWUSXsEJgQkUAUGdFFJRCqBzHQPQYU+WJFUsQp1pa64VeHR6mDQp82dtL/FbRrjCVnPOV+NEqHZ9pPWbZKR9J1w6WIb8ObG5B/xjB4Wj203VEeKVZwj07JK0mOC7XpszPJo/Xf4ehimDtB4u+kEYLDG1evd+FeKhUXXFAXOc5uNLpCD4VanYu3hOGz2EI1aMmRerujmXTCLWdk7U+7XtowI93OH2YzA4mxFTpCXiSRjXaJbgZtFdLuKw0SOQKEQ1eJ86xXwq2YVrkUBi9bOmrV/IP1E40xcNSU6mPOx17y1Z8+dxetmmy2aYu1RUnxVHbrBDDCOMSkvzYXcC3JipqyHD6swl/OJR1rdngN78ZAxWQp8dmb4tsdsNunETY9jUQNuYSMpIP88U5RqLyiQYSOlhde4SJKmgvJrRlJyGU6Vxc9azPDP2DY6WM3v6dNjYNvQAwWJpNC4UhGNKc/pOpaUQUB4pJ/YLqO4Io2qdZHNCZeDBWisdMu94Ua/xxnehdNrO6LEW722+bykcyM7MgU0R8h+rgTlYVnTeB9vpACGot1FGG14rY7QNN1nVe1SWdpXDH4WIhpa8Pg4awZ G13PuK0B ehTJRrjlB30GHZZ+wO2Rn+/o1WZVvdVPaf2hNpVqZhjinj2lIZwxp82iKJ/0nV+AbBTnkkusGRSPY+6npqI0V5fifxdqaU8jphh9T0P24j1u0OCERnzYZNW+06PZmgYfdawsntnvHTY6mlID9Unn1Z/uwZMNRAX3kBJLREAvNp/WHy1l0isvPO4Ionu9cwIanC3ixuyDEjxoAcMAImyl1Jm+difMAcjBXTjssgLZC6YP2MvPkt+S5VQp92MxUk45+T1wfuOBVKtPeCFGniXQNSKMvkKglZJLhQalx/ElkRoFfMcL7kdJT/mlNz/rzT9UKS7dBzFbsJiO6tcdMJq5Adif82b7M/jGg22IJHuzx/FdgbbI= 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: List-Subscribe: List-Unsubscribe: On 2024/7/1 19:12, Baolin Wang wrote: > > > On 2024/7/1 17:43, Bang Li wrote: >> Hi, Baolin >> >> On 2024/7/1 16:33, Baolin Wang wrote: >>> >>> >>> On 2024/7/1 15:55, Ryan Roberts wrote: >>>> On 28/06/2024 11:49, Bang Li wrote: >>>>> After the commit 7fb1b252afb5 ("mm: shmem: add mTHP support for >>>>> anonymous shmem"), we can configure different policies through >>>>> the multi-size THP sysfs interface for anonymous shmem. But >>>>> currently "THPeligible" indicates only whether the mapping is >>>>> eligible for allocating THP-pages as well as the THP is PMD >>>>> mappable or not for anonymous shmem, we need to support semantics >>>>> for mTHP with anonymous shmem similar to those for mTHP with >>>>> anonymous memory. >>>>> >>>>> Signed-off-by: Bang Li >>>>> --- >>>>>   fs/proc/task_mmu.c      | 10 +++++++--- >>>>>   include/linux/huge_mm.h | 11 +++++++++++ >>>>>   mm/shmem.c              |  9 +-------- >>>>>   3 files changed, 19 insertions(+), 11 deletions(-) >>>>> >>>>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c >>>>> index 93fb2c61b154..09b5db356886 100644 >>>>> --- a/fs/proc/task_mmu.c >>>>> +++ b/fs/proc/task_mmu.c >>>>> @@ -870,6 +870,7 @@ static int show_smap(struct seq_file *m, void *v) >>>>>   { >>>>>       struct vm_area_struct *vma = v; >>>>>       struct mem_size_stats mss = {}; >>>>> +    bool thp_eligible; >>>>>       smap_gather_stats(vma, &mss, 0); >>>>> @@ -882,9 +883,12 @@ static int show_smap(struct seq_file *m, void *v) >>>>>       __show_smap(m, &mss, false); >>>>> -    seq_printf(m, "THPeligible:    %8u\n", >>>>> -           !!thp_vma_allowable_orders(vma, vma->vm_flags, >>>>> -               TVA_SMAPS | TVA_ENFORCE_SYSFS, THP_ORDERS_ALL)); >>>>> +    thp_eligible = !!thp_vma_allowable_orders(vma, vma->vm_flags, >>>>> +                        TVA_SMAPS | TVA_ENFORCE_SYSFS, >>>>> THP_ORDERS_ALL); >>>>> +    if (vma_is_anon_shmem(vma)) >>>>> +        thp_eligible = >>>>> !!shmem_allowable_huge_orders(file_inode(vma->vm_file), >>>>> +                            vma, vma->vm_pgoff, thp_eligible); >>>> >>>> Afraid I haven't been following the shmem mTHP support work as much >>>> as I would >>>> have liked, but is there a reason why we need a separate function >>>> for shmem? >>> >>> Since shmem_allowable_huge_orders() only uses shmem specific logic to >>> determine if huge orders are allowable, there is no need to >>> complicate the thp_vma_allowable_orders() function by adding more >>> shmem related logic, making it more bloated. In my view, providing a >>> dedicated helper shmem_allowable_huge_orders(), specifically for >>> shmem, simplifies the logic. >>> >>> IIUC, I agree with David's suggestion that the >>> shmem_allowable_huge_orders() helper function could be used in >>> thp_vma_allowable_orders() to support shmem mTHP. Something like: >>> >>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >>> index c7ce28f6b7f3..9677fe6cf478 100644 >>> --- a/mm/huge_memory.c >>> +++ b/mm/huge_memory.c >>> @@ -151,10 +151,13 @@ unsigned long __thp_vma_allowable_orders(struct >>> vm_area_struct *vma, >>>           * Must be done before hugepage flags check since shmem has its >>>           * own flags. >>>           */ >>> -       if (!in_pf && shmem_file(vma->vm_file)) >>> -               return shmem_is_huge(file_inode(vma->vm_file), >>> vma->vm_pgoff, >>> -                                    !enforce_sysfs, vma->vm_mm, >>> vm_flags) >>> -                       ? orders : 0; >>> +       if (!in_pf && shmem_file(vma->vm_file)) { >>> +               bool global_huge = >>> shmem_is_huge(file_inode(vma->vm_file), vma->vm_pgoff, >>> +                                    !enforce_sysfs, vma->vm_mm, >>> vm_flags); >>> + >>> +               return >>> shmem_allowable_huge_orders(file_inode(vma->vm_file), >>> +                                       vma, vma->vm_pgoff, >>> global_huge); >>> +       } >>> >>>          if (!vma_is_anonymous(vma)) { >>>                  /* >>> >>>> Couldn't (shouldn't) thp_vma_allowable_orders() be taught to handle >>>> shmem too? >>>> >>>>> +    seq_printf(m, "THPeligible:    %8u\n", thp_eligible); >>>>>       if (arch_pkeys_enabled()) >>>>>           seq_printf(m, "ProtectionKey:  %8u\n", vma_pkey(vma)); >>>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h >>>>> index 212cca384d7e..f87136f38aa1 100644 >>>>> --- a/include/linux/huge_mm.h >>>>> +++ b/include/linux/huge_mm.h >>>>> @@ -267,6 +267,10 @@ unsigned long thp_vma_allowable_orders(struct >>>>> vm_area_struct *vma, >>>>>       return __thp_vma_allowable_orders(vma, vm_flags, tva_flags, >>>>> orders); >>>>>   } >>>>> +unsigned long shmem_allowable_huge_orders(struct inode *inode, >>>>> +                struct vm_area_struct *vma, pgoff_t index, >>>>> +                bool global_huge); >>>>> + >>>>>   struct thpsize { >>>>>       struct kobject kobj; >>>>>       struct list_head node; >>>>> @@ -460,6 +464,13 @@ static inline unsigned long >>>>> thp_vma_allowable_orders(struct vm_area_struct *vma, >>>>>       return 0; >>>>>   } >>>>> +static inline unsigned long shmem_allowable_huge_orders(struct >>>>> inode *inode, >>>>> +                struct vm_area_struct *vma, pgoff_t index, >>>>> +                bool global_huge) >>>>> +{ >>>>> +    return 0; >>>>> +} >>>>> + >>>>>   #define transparent_hugepage_flags 0UL >>>>>   #define thp_get_unmapped_area    NULL >>>>> diff --git a/mm/shmem.c b/mm/shmem.c >>>>> index d495c0701a83..aa85df9c662a 100644 >>>>> --- a/mm/shmem.c >>>>> +++ b/mm/shmem.c >>>>> @@ -1622,7 +1622,7 @@ static gfp_t limit_gfp_mask(gfp_t huge_gfp, >>>>> gfp_t limit_gfp) >>>>>   } >>>>>   #ifdef CONFIG_TRANSPARENT_HUGEPAGE >>>>> -static unsigned long shmem_allowable_huge_orders(struct inode *inode, >>>>> +unsigned long shmem_allowable_huge_orders(struct inode *inode, >>>>>                   struct vm_area_struct *vma, pgoff_t index, >>>>>                   bool global_huge) >>>>>   { >>>>> @@ -1707,13 +1707,6 @@ static unsigned long >>>>> shmem_suitable_orders(struct inode *inode, struct vm_fault >>>>>       return orders; >>>>>   } >>>>>   #else >>>>> -static unsigned long shmem_allowable_huge_orders(struct inode *inode, >>>>> -                struct vm_area_struct *vma, pgoff_t index, >>>>> -                bool global_huge) >>>>> -{ >>>>> -    return 0; >>>>> -} >>>>> - >>>>>   static unsigned long shmem_suitable_orders(struct inode *inode, >>>>> struct vm_fault *vmf, >>>>>                          struct address_space *mapping, pgoff_t index, >>>>>                          unsigned long orders) >> >> Thanks for the reference code. Currently, we only implement the mTHP of >> anonymous shmem, so we only need to handle anonymous shmem specially. As >> shown in the following code: >> >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -151,10 +151,14 @@ unsigned long __thp_vma_allowable_orders(struct >> vm_area_struct *vma, >>           * Must be done before hugepage flags check since shmem has its >>           * own flags. >>           */ >> -       if (!in_pf && shmem_file(vma->vm_file)) >> -               return shmem_is_huge(file_inode(vma->vm_file), >> vma->vm_pgoff, >> -                                    !enforce_sysfs, vma->vm_mm, >> vm_flags) >> -                       ? orders : 0; >> +       if (!in_pf && shmem_file(vma->vm_file)) { >> +               bool global_huge = >> shmem_is_huge(file_inode(vma->vm_file), vma->vm_pgoff, >> +                                    !enforce_sysfs, vma->vm_mm, >> vm_flags); > > Nit: add a blank line after the declaration. Otherwise looks good to me. It doesn't matter to me whether I add spaces or not, thanks for your suggestion anyway. Thanks, Bang > >> +               if (!vma_is_anon_shmem(vma)) >> +                       return global_huge? orders : 0; >> +               return >> shmem_allowable_huge_orders(file_inode(vma->vm_file), >> +                                               vma, vma->vm_pgoff, >> global_huge); >> +       } >> >>          if (!vma_is_anonymous(vma)) { >> >> Thanks, >> Bang