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 99F2FC433EF for ; Tue, 12 Jul 2022 17:28:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 274639400BB; Tue, 12 Jul 2022 13:28:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1FBE3940063; Tue, 12 Jul 2022 13:28:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0C3D39400BB; Tue, 12 Jul 2022 13:28:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id EEB92940063 for ; Tue, 12 Jul 2022 13:28:01 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id CA05D350B5 for ; Tue, 12 Jul 2022 17:28:01 +0000 (UTC) X-FDA: 79679130762.10.60C5FF0 Received: from mail-pf1-f173.google.com (mail-pf1-f173.google.com [209.85.210.173]) by imf28.hostedemail.com (Postfix) with ESMTP id 5A4A2C0072 for ; Tue, 12 Jul 2022 17:28:01 +0000 (UTC) Received: by mail-pf1-f173.google.com with SMTP id x184so8056863pfx.2 for ; Tue, 12 Jul 2022 10:28:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4iqOuSHLaJoDHR/8ynoc1v4OCSXMy/aPsHs9FlAbrzI=; b=Yj1mvU/k1Tpo9qFfRcJkS6UI3k9Yd++lv+fihYQzkMu455nbtgLsxpj6aK1HF2ObnG adEfqJsIisRa9kpkxVbcY/Vunx8tF7VU+IGt82j72U4IM2Je3u3RbczKcM8uUCJysiuO wm216b9OfCzMhum2aUhrgwKCCx/DHOxw1K9PHY3oflZGUi/UJw6D8sDwHOqy9BP9IpN0 BlbHdKivPunY96f/Om118numW8ez5JhdkXgGBd7GGWveZDelYGumW7mQw7EFcoj1/3Cw PtlrvC8IkcGWD0Knz7qKr+Mce3F3Xof1ZnOyISwlHyqP6uEulpqh8pVFOOAysQatRZp2 8PXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4iqOuSHLaJoDHR/8ynoc1v4OCSXMy/aPsHs9FlAbrzI=; b=vH3XZu6eEK+SJkfxm/GUmRaSWrw7aDWqNvibwCsDZPtQ45AOxDq7OGEJo0MzOF1DPw pno8Y87UTR7NN8CB9qpVAWaeItYYeKzJYopvvW3EuV3Nx9MqG3YSXksAVGiyL7JTKIns 9Dp1LpcIqrnGM3j0qflsIJTwtNQGBLGOxUD6IoLAmkd8ViDRKl5tVDdzkuO6+dIt4yrt rF2KSC6jePwzS8IKZX4dDIDUCOx+xPztq4XneP4r4cq/BwO5PAOcj0p4cz1mN1+BbcmN f+QKny31l9vuw3ZmcLo3qnnDraSzMCLij5l0o1lpN/4Z/IQi90Dbq53mDQ1cMImy+PPN M1ew== X-Gm-Message-State: AJIora/qxbicgTHNO2ntc6xMDhiHTWyqaz7Tah2ywt3ut69p/NP1VYrl D7zsHXjEVJ43zChm8vp7lIvZmePvI3OhuGf2rK4= X-Google-Smtp-Source: AGRyM1uImdlmrVx4w+UwALCFkOZqzTKg65+2J3mGVOrhWtnmt/6lU6dzbkoVUgDpIl5BkWLM3Vyvh+CuYH67mHt9i3o= X-Received: by 2002:a05:6a00:1a0e:b0:52a:cef3:b4a1 with SMTP id g14-20020a056a001a0e00b0052acef3b4a1mr11971587pfv.23.1657646880347; Tue, 12 Jul 2022 10:28:00 -0700 (PDT) MIME-Version: 1.0 References: <20220706235936.2197195-1-zokeefe@google.com> <20220706235936.2197195-14-zokeefe@google.com> In-Reply-To: From: Yang Shi Date: Tue, 12 Jul 2022 10:27:47 -0700 Message-ID: Subject: Re: [mm-unstable v7 13/18] proc/smaps: add PMDMappable field to smaps To: "Zach O'Keefe" Cc: Alex Shi , David Hildenbrand , David Rientjes , Matthew Wilcox , Michal Hocko , Pasha Tatashin , Peter Xu , Rongwei Wang , SeongJae Park , Song Liu , Vlastimil Babka , Zi Yan , Linux MM , Andrea Arcangeli , Andrew Morton , Arnd Bergmann , Axel Rasmussen , Chris Kennelly , Chris Zankel , Helge Deller , Hugh Dickins , Ivan Kokshaysky , "James E.J. Bottomley" , Jens Axboe , "Kirill A. Shutemov" , Matt Turner , Max Filippov , Miaohe Lin , Minchan Kim , Patrick Xia , Pavel Begunkov , Thomas Bogendoerfer , James Houghton Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1657646881; a=rsa-sha256; cv=none; b=6n9PNaDSvDHyD4MktOJsKT5wH7fj+ud2Cr8LPEFMZX79UDA4dmCltQFAyGoP9ALo5n0FeE srbaM7buCVWRgnh0hsZDSCHchrEz07TAyVu1jRFfU8Vt8q32YPkUilQEpsQEmivc5thO8B +HEhO0VS/KxPE7cO7nE6spfcr50AJVg= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="Yj1mvU/k"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf28.hostedemail.com: domain of shy828301@gmail.com designates 209.85.210.173 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=1657646881; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=4iqOuSHLaJoDHR/8ynoc1v4OCSXMy/aPsHs9FlAbrzI=; b=NQCNJZSvLQDYmpEJK11pS25ubZuOooUpOlnCRuwzdjA6qD9GRjvvxsQrs0vMeaTPezZ8Mj 6kikEJSS1U+tPBUUifiSGPHUB1iCSoZRXGId6oZeexMRHSpOjQBDvDvCFTfG5zNRlWyTwb 3+fN4B2GX1N9cDLAWgGbrqJJutwvQYI= X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 5A4A2C0072 Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="Yj1mvU/k"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf28.hostedemail.com: domain of shy828301@gmail.com designates 209.85.210.173 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Stat-Signature: pn64hc5dou6u5zsmmiz9fpf8gw3xyhgi X-Rspam-User: X-HE-Tag: 1657646881-485537 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, Jul 12, 2022 at 9:31 AM Zach O'Keefe wrote: > > On Jul 11 14:37, Yang Shi wrote: > > On Wed, Jul 6, 2022 at 5:06 PM Zach O'Keefe wrote: > > > > > > Add PMDMappable field to smaps output which informs the user if memory > > > in the VMA can be PMD-mapped by MADV_COLLAPSE. > > > > > > The distinction from THPeligible is needed for two reasons: > > > > > > 1) For THP, MADV_COLLAPSE is not coupled to THP sysfs controls, which > > > THPeligible reports. > > > > > > 2) PMDMappable can also be used in HugeTLB fine-granularity mappings, > > > which are independent from THP. > > > > Could you please elaborate the usecase? The user checks this hint > > before calling MADV_COLLAPSE? Is it really necessary? > > > > And, TBH it sounds confusing and we don't have to maintain both > > THPeligible and PMDMappable. We could just relax THPeligible to make > > it return 1 even though THP is disabled by sysfs but MADV_COLLAPSE > > could collapse it if such hint is useful. > > > > Hey Yang, > > Thanks for taking the time to review this series again, and thanks for > challenging this. > > TLDR: "Is it really necessary" - at the moment, no, probably not .. but I think > it's "useful". > > Rationale: > > 1. IMO, I thought was was confusing seeing: > > ... > AnonHugePages: 2048 kB > ShmemPmdMapped: 0 kB > FilePmdMapped: 0 kB > Shared_Hugetlb: 0 kB > Private_Hugetlb: 0 kB > Swap: 0 kB > SwapPss: 0 kB > Locked: 0 kB > THPeligible: 0 > ... > > Maybe this could simply be clarified in the docs though. I guess we can already > get: > > ... > AnonHugePages: 0 kB > ShmemPmdMapped: 0 kB > FilePmdMapped: 2048 kB > Shared_Hugetlb: 0 kB > Private_Hugetlb: 0 kB > Swap: 0 kB > SwapPss: 0 kB > Locked: 0 kB > THPeligible: 0 > ... > > today[1], so perhaps it's not a big deal. Not only that, if you have file PMD mapped then turn the THP sysfs flag off, you get the same result. It is just a hint and just shows the status at that moment when reading smaps. > > > 2. It was useful for debugging - similar to rationale for including > THPeligible1[2], the logic for determining if a VMA is eligible is pretty > complicated. I.e. is this file mapped suitably? Unlike THPeligible, however, > madvise(2) has the ability to set errno on failure to help* diagnose why some > memory isn't being backed. I don't disagree it would help for debugging. But as a user who doesn't know too much about kernel internals, when I see THPeligible and PMDmappable, I would get confused TBH. And do we have to maintain another similar hint? Maybe not. > > 3. For the immediately-envisioned usecases, the user "knows" about what memory > they are acting on. However, eventually we'd like to experiment with moving THP > utilization policy to userspace. Here, it would be useful if the userspace agent > managing was made aware of what memory it should be managing. I don't have a > working prototype of what this would like yet, however. It is not a strong justification to add some user visible stuff for a future feature (not even prototyped) since things may change, it sounds safer to add such things once the usecase is solid TBH. > > 4. I thought it was neat that this field could be reused for HugeTLB > fine-granularity mappings - but TBH I'm not sure how useful it'd be there. > > I figured relaxing existing THPeligible could break existing users / tests, and > it'd be likewise confusing for them to see THPeligible: 1, but then have faults > fail and they'd then have to go check sysfs settings and vma flags ; we'd be > back in pre-commit 7635d9cbe832 ("mm, thp, proc: report THP eligibility for each > vma"). I'm not sure what applications rely on this hint, but if they are just some test scripts, I think it should be fine. I don't think we guarantee the test scripts won't get broken. AFAIK some test scripts rely on the kernel dmesg text, for example, OOMs. And the meaning of the fields do change, for example, inactive anon of /proc/meminfo, which was changed by the patchset which put anon pages on inactive list first instead of active list. We already noticed the abnormal value from our monitoring tool when we adopted 5.10+ kernel. And /proc/vmstat also had some fields renamed, for example, workingset_refault of /proc/vmstat, it was split to workseting_refault_anon and workingset_refault_file, so we had to update our monitoring scripts accordingly. I think /proc/meminfo and /proc/vmstat are more heavily used than smaps. > > Thanks, > Zach > > [1] https://lore.kernel.org/linux-mm/YrxbQGiwml24APCx@google.com/ > > > > > > > > > > Signed-off-by: Zach O'Keefe > > > --- > > > Documentation/filesystems/proc.rst | 10 ++++++++-- > > > fs/proc/task_mmu.c | 2 ++ > > > 2 files changed, 10 insertions(+), 2 deletions(-) > > > > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > > > index 47e95dbc820d..f207903a57a5 100644 > > > --- a/Documentation/filesystems/proc.rst > > > +++ b/Documentation/filesystems/proc.rst > > > @@ -466,6 +466,7 @@ Memory Area, or VMA) there is a series of lines such as the following:: > > > MMUPageSize: 4 kB > > > Locked: 0 kB > > > THPeligible: 0 > > > + PMDMappable: 0 > > > VmFlags: rd ex mr mw me dw > > > > > > The first of these lines shows the same information as is displayed for the > > > @@ -518,9 +519,14 @@ replaced by copy-on-write) part of the underlying shmem object out on swap. > > > does not take into account swapped out page of underlying shmem objects. > > > "Locked" indicates whether the mapping is locked in memory or not. > > > > > > +"PMDMappable" indicates if the memory can be mapped by PMDs - 1 if true, 0 > > > +otherwise. It just shows the current status. Note that this is memory > > > +operable on explicitly by MADV_COLLAPSE. > > > + > > > "THPeligible" indicates whether the mapping is eligible for allocating THP > > > -pages as well as the THP is PMD mappable or not - 1 if true, 0 otherwise. > > > -It just shows the current status. > > > +pages by the kernel, as well as the THP is PMD mappable or not - 1 if true, 0 > > > +otherwise. It just shows the current status. Note this is memory the kernel can > > > +transparently provide as THPs. > > > > > > "VmFlags" field deserves a separate description. This member represents the > > > kernel flags associated with the particular virtual memory area in two letter > > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > > > index f8cd58846a28..29f2089456ba 100644 > > > --- a/fs/proc/task_mmu.c > > > +++ b/fs/proc/task_mmu.c > > > @@ -867,6 +867,8 @@ static int show_smap(struct seq_file *m, void *v) > > > > > > seq_printf(m, "THPeligible: %d\n", > > > hugepage_vma_check(vma, vma->vm_flags, true, false, true)); > > > + seq_printf(m, "PMDMappable: %d\n", > > > + hugepage_vma_check(vma, vma->vm_flags, true, false, false)); > > > > > > if (arch_pkeys_enabled()) > > > seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma)); > > > -- > > > 2.37.0.rc0.161.g10f37bed90-goog > > >