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 2148CC433EF for ; Fri, 10 Jun 2022 21:24:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 666F88D00E5; Fri, 10 Jun 2022 17:24:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5C8228D00E2; Fri, 10 Jun 2022 17:24:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 442638D00E5; Fri, 10 Jun 2022 17:24:48 -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 2F2178D00E2 for ; Fri, 10 Jun 2022 17:24:48 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id E8AEF35B67 for ; Fri, 10 Jun 2022 21:24:47 +0000 (UTC) X-FDA: 79563605814.11.1E58418 Received: from mail-pj1-f52.google.com (mail-pj1-f52.google.com [209.85.216.52]) by imf02.hostedemail.com (Postfix) with ESMTP id 90B0D80083 for ; Fri, 10 Jun 2022 21:24:47 +0000 (UTC) Received: by mail-pj1-f52.google.com with SMTP id e9so512373pju.5 for ; Fri, 10 Jun 2022 14:24:47 -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=hcHzsQVZ+PTFSV9X4stAy+tj1JmFLmJSZcQbEdxANTg=; b=ArwMa9bZf+r8xebzbmSfcJnbMlpoqRyzcSgvj7aIl8Q4FjIUw7aXaf38oXRlRSn7nR Jz898ErOjc0pJrCZEZMVBKGoR3C1AdzLDeXHhUZh86b24KW7czKHTssyDKE8LjkoujJU gaZxcgqe82On1qlRiuM4PNX+9W2XHJ/eIi3YMMoliaWGZXaLn8b6x4IhHMDyo+esBaNM l9XuoowBf0U8oJM+I0zVZGYfezI0mMNz3uT6LemGDhOZwVWLNVbH3Mm/cQs8kHcGJXtm V/HLKcHwguRzsTFMvHpaT4XobPXwc57PkkUzh8A88+pMeLItBkhOueNsP/exglZXyPQS IDeg== 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=hcHzsQVZ+PTFSV9X4stAy+tj1JmFLmJSZcQbEdxANTg=; b=SKVfLzSSJZMvQqoWecicR7Cjq4Cl/JBpjn1Xec2ifNyS8lePbk7g/kCEwRjVpNb/KM 52X+8IdjR8aekCv30NZBUg8GLZKSznnb50NCmAPnKM0K6hTVwXcfHfnIQgU5qmfw132C kiRzDMb2imjY3DqyBgOZy+6m+V0vl4MakW0/kjORrssT3i5ekGFert6Rt4S59wsrccyV EdVq/Tnf5YQyswyh92hGhf1vA4mKUgAOca8DSiwKjzsuZXF3+6PAofhPz6BxgB+OBgRS RfEiuX9V3jv7sgQoV/EQ2CZVNdo9SOnatC2v/agbjrYSOYU1RRQQduKNqLc2xtr8CPCY FU4Q== X-Gm-Message-State: AOAM530u+hrMcAUwTN9PcOb6Mb/YMF93mb1g3hHVZ72dl/Eb5Po5SSpg TXcPeID+bPByJjSl5/fEt+2XHbwxoRQuGsDvSp43/9Vv X-Google-Smtp-Source: ABdhPJzzagpZPkwig2/NpWC77DWNNuvXz/zd67odgxEz2hdg1cuYtctz2IVGfqEqz7OatcqOxiO3RK25CjEEXEb6jqk= X-Received: by 2002:a17:90a:de15:b0:1df:63dd:9cfc with SMTP id m21-20020a17090ade1500b001df63dd9cfcmr1703274pjv.200.1654896286440; Fri, 10 Jun 2022 14:24:46 -0700 (PDT) MIME-Version: 1.0 References: <20220606214414.736109-1-shy828301@gmail.com> <20220606214414.736109-3-shy828301@gmail.com> In-Reply-To: From: Yang Shi Date: Fri, 10 Jun 2022 14:24:34 -0700 Message-ID: Subject: Re: [v3 PATCH 2/7] mm: thp: introduce transhuge_vma_size_ok() helper To: "Zach O'Keefe" Cc: Vlastimil Babka , "Kirill A. Shutemov" , Matthew Wilcox , Andrew Morton , Linux MM , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1654896287; 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=hcHzsQVZ+PTFSV9X4stAy+tj1JmFLmJSZcQbEdxANTg=; b=aKIN9Y7EFzopIU5bxEcd2JPOtxI5shei1bgNYJQyMj41UgULdTzgO6BhiyA3LX5c23vPwh ae8nvr1kzxkXyH+DD6Cf/O0NWRRBQKfT5CmFxwLhgm7b4UbQBtJLtLZ8+1jWBQZ7iLB4jL 1w8rN09T4vqxyeIlmiTXNScFqySiDbM= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1654896287; a=rsa-sha256; cv=none; b=O7sARuNg6fIysd+WfjxIkAvUV1K7W/za3Mt/Qwm67goHJU4w/Bxus6DyafhJspkCtlZk9I 1zDXkhTbI9AQouBEr3atRaN0wHxCaTIvA9sMiKqEzzAyDSy9XojLJmbwb7EXHdvzuNRRsZ e4O61IkeXvOjRi0KrLoZYoH6djmVMc0= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ArwMa9bZ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf02.hostedemail.com: domain of shy828301@gmail.com designates 209.85.216.52 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Rspamd-Server: rspam11 X-Rspam-User: Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ArwMa9bZ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf02.hostedemail.com: domain of shy828301@gmail.com designates 209.85.216.52 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Stat-Signature: ggsbpfr6m6abxfrci4576ikb8doop93g X-Rspamd-Queue-Id: 90B0D80083 X-HE-Tag: 1654896287-955536 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 Fri, Jun 10, 2022 at 9:38 AM Yang Shi wrote: > > On Thu, Jun 9, 2022 at 5:52 PM Zach O'Keefe wrote: > > > > On Thu, Jun 9, 2022 at 5:08 PM Yang Shi wrote: > > > > > > On Thu, Jun 9, 2022 at 3:21 PM Zach O'Keefe wrote: > > > > > > > > On Mon, Jun 6, 2022 at 2:44 PM Yang Shi wrote: > > > > > > > > > > There are couple of places that check whether the vma size is ok for > > > > > THP or not, they are open coded and duplicate, introduce > > > > > transhuge_vma_size_ok() helper to do the job. > > > > > > > > > > Signed-off-by: Yang Shi > > > > > --- > > > > > include/linux/huge_mm.h | 17 +++++++++++++++++ > > > > > mm/huge_memory.c | 5 +---- > > > > > mm/khugepaged.c | 12 ++++++------ > > > > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > > > > > > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > > > > index 648cb3ce7099..a8f61db47f2a 100644 > > > > > --- a/include/linux/huge_mm.h > > > > > +++ b/include/linux/huge_mm.h > > > > > @@ -116,6 +116,18 @@ extern struct kobj_attribute shmem_enabled_attr; > > > > > > > > > > extern unsigned long transparent_hugepage_flags; > > > > > > > > > > +/* > > > > > + * The vma size has to be large enough to hold an aligned HPAGE_PMD_SIZE area. > > > > > + */ > > > > > +static inline bool transhuge_vma_size_ok(struct vm_area_struct *vma) > > > > > +{ > > > > > + if (round_up(vma->vm_start, HPAGE_PMD_SIZE) < > > > > > + (vma->vm_end & HPAGE_PMD_MASK)) > > > > > + return true; > > > > > + > > > > > + return false; > > > > > +} > > > > > > > > First time coming across round_up() - thanks for that - but for > > > > symmetry, maybe also use round_down() for the end? No strong opinion - > > > > just a suggestion given I've just discovered it. > > > > > > Yeah, round_down is fine too. > > > > > > > > > > > > static inline bool transhuge_vma_suitable(struct vm_area_struct *vma, > > > > > unsigned long addr) > > > > > { > > > > > @@ -345,6 +357,11 @@ static inline bool transparent_hugepage_active(struct vm_area_struct *vma) > > > > > return false; > > > > > } > > > > > > > > > > +static inline bool transhuge_vma_size_ok(struct vm_area_struct *vma) > > > > > +{ > > > > > + return false; > > > > > +} > > > > > + > > > > > static inline bool transhuge_vma_suitable(struct vm_area_struct *vma, > > > > > unsigned long addr) > > > > > { > > > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > > > > index 48182c8fe151..36ada544e494 100644 > > > > > --- a/mm/huge_memory.c > > > > > +++ b/mm/huge_memory.c > > > > > @@ -71,10 +71,7 @@ unsigned long huge_zero_pfn __read_mostly = ~0UL; > > > > > > > > > > bool transparent_hugepage_active(struct vm_area_struct *vma) > > > > > { > > > > > - /* The addr is used to check if the vma size fits */ > > > > > - unsigned long addr = (vma->vm_end & HPAGE_PMD_MASK) - HPAGE_PMD_SIZE; > > > > > - > > > > > - if (!transhuge_vma_suitable(vma, addr)) > > > > > + if (!transhuge_vma_size_ok(vma)) > > > > > return false; > > > > > if (vma_is_anonymous(vma)) > > > > > return __transparent_hugepage_enabled(vma); > > > > > > > > Do we need a check for vma->vm_pgoff alignment here, after > > > > !vma_is_anonymous(), and now that we don't call > > > > transhuge_vma_suitable()? > > > > > > Actually I was thinking about this too. But the THPeligible bit shown > > > by smaps is a little bit ambiguous for file vma. The document says: > > > "THPeligible" indicates whether the mapping is eligible for allocating > > > THP pages - 1 if true, 0 otherwise. > > > > > > Even though it doesn't fulfill the alignment, it is still possible to > > > get THP allocated, but just can't be PMD mapped. So the old behavior > > > of THPeligible for file vma seems problematic, or at least doesn't > > > match the document. > > > > I think the term "THP" is used ambiguously. Often, but not always, in > > the code, folks will go out of their way to specify "hugepage-sized" > > page vs "pmd-mapped hugepage" - but at least from my experience, > > external documentation doesn't. Given that THP as a concept doesn't > > make much sense without the possibility of pmd-mapping, I think > > "THPeligible here means "pmd mappable". For example, AnonHugePages in > > smaps means pmd-mapped anon hugepages. > > Yeah, depends on the expectation. The funny thing is I was the last one who touched the THPeligible. It seems the document needs to be updated too to make "pmd mappable" more explicitly. > > > > > That all said - the following patches will delete > > transparent_hugepage_active() anyways. > > Yes, how I could forget this :-( The following removal of > transparent_hugepage_active() will restore the old behavior. > > > > > > I should elaborate this in the commit log. > > > > > > > > > > > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > > > > > index 84b9cf4b9be9..d0f8020164fc 100644 > > > > > --- a/mm/khugepaged.c > > > > > +++ b/mm/khugepaged.c > > > > > @@ -454,6 +454,9 @@ bool hugepage_vma_check(struct vm_area_struct *vma, > > > > > vma->vm_pgoff, HPAGE_PMD_NR)) > > > > > return false; > > > > > > > > > > + if (!transhuge_vma_size_ok(vma)) > > > > > + return false; > > > > > + > > > > > /* Enabled via shmem mount options or sysfs settings. */ > > > > > if (shmem_file(vma->vm_file)) > > > > > return shmem_huge_enabled(vma); > > > > > @@ -512,9 +515,7 @@ void khugepaged_enter_vma(struct vm_area_struct *vma, > > > > > unsigned long vm_flags) > > > > > { > > > > > if (!test_bit(MMF_VM_HUGEPAGE, &vma->vm_mm->flags) && > > > > > - khugepaged_enabled() && > > > > > - (((vma->vm_start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK) < > > > > > - (vma->vm_end & HPAGE_PMD_MASK))) { > > > > > + khugepaged_enabled()) { > > > > > if (hugepage_vma_check(vma, vm_flags)) > > > > > __khugepaged_enter(vma->vm_mm); > > > > > } > > > > > @@ -2142,10 +2143,9 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, > > > > > progress++; > > > > > continue; > > > > > } > > > > > - hstart = (vma->vm_start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK; > > > > > + > > > > > + hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE); > > > > > hend = vma->vm_end & HPAGE_PMD_MASK; > > > > > - if (hstart >= hend) > > > > > - goto skip; > > > > > if (khugepaged_scan.address > hend) > > > > > goto skip; > > > > > if (khugepaged_scan.address < hstart) > > > > > > > > Likewise, could do round_down() here (just a suggestion) > > > > > > Fine to me. > > > > > > > > > > > > -- > > > > > 2.26.3 > > > > > > > > > >