linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nico Pache <npache@redhat.com>
To: Baolin Wang <baolin.wang@linux.alibaba.com>,
	David Rientjes <rientjes@google.com>,
	 zokeefe@google.com
Cc: linux-mm@kvack.org, linux-doc@vger.kernel.org,
	 linux-kernel@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org,  david@redhat.com,
	ziy@nvidia.com, lorenzo.stoakes@oracle.com,
	 Liam.Howlett@oracle.com, ryan.roberts@arm.com, dev.jain@arm.com,
	 corbet@lwn.net, rostedt@goodmis.org, mhiramat@kernel.org,
	 mathieu.desnoyers@efficios.com, akpm@linux-foundation.org,
	baohua@kernel.org,  willy@infradead.org, peterx@redhat.com,
	wangkefeng.wang@huawei.com,  usamaarif642@gmail.com,
	sunnanyong@huawei.com, vishal.moola@gmail.com,
	 thomas.hellstrom@linux.intel.com, yang@os.amperecomputing.com,
	 kirill.shutemov@linux.intel.com, aarcange@redhat.com,
	raquini@redhat.com,  anshuman.khandual@arm.com,
	catalin.marinas@arm.com, tiwai@suse.de,  will@kernel.org,
	dave.hansen@linux.intel.com, jack@suse.cz, cl@gentwo.org,
	 jglisse@google.com, surenb@google.com, hannes@cmpxchg.org,
	mhocko@suse.com,  rdunlap@infradead.org
Subject: Re: [PATCH v7 06/12] khugepaged: introduce khugepaged_scan_bitmap for mTHP support
Date: Wed, 21 May 2025 04:23:09 -0600	[thread overview]
Message-ID: <CAA1CXcAWcahkxzsvK_bcWei6or_gKBjt+97dqhuSem8N7cBAQw@mail.gmail.com> (raw)
In-Reply-To: <ed1d1281-ece3-4d2c-8e58-aaeb436d3927@linux.alibaba.com>

On Tue, May 20, 2025 at 4:09 AM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
> Sorry for late reply.
>
> On 2025/5/17 14:47, Nico Pache wrote:
> > On Thu, May 15, 2025 at 9:20 PM Baolin Wang
> > <baolin.wang@linux.alibaba.com> wrote:
> >>
> >>
> >>
> >> On 2025/5/15 11:22, Nico Pache wrote:
> >>> khugepaged scans anons PMD ranges for potential collapse to a hugepage.
> >>> To add mTHP support we use this scan to instead record chunks of utilized
> >>> sections of the PMD.
> >>>
> >>> khugepaged_scan_bitmap uses a stack struct to recursively scan a bitmap
> >>> that represents chunks of utilized regions. We can then determine what
> >>> mTHP size fits best and in the following patch, we set this bitmap while
> >>> scanning the anon PMD. A minimum collapse order of 2 is used as this is
> >>> the lowest order supported by anon memory.
> >>>
> >>> max_ptes_none is used as a scale to determine how "full" an order must
> >>> be before being considered for collapse.
> >>>
> >>> When attempting to collapse an order that has its order set to "always"
> >>> lets always collapse to that order in a greedy manner without
> >>> considering the number of bits set.
> >>>
> >>> Signed-off-by: Nico Pache <npache@redhat.com>
> >>
> >> Sigh. You still haven't addressed or explained the issues I previously
> >> raised [1], so I don't know how to review this patch again...
> > Can you still reproduce this issue?
>
> Yes, I can still reproduce this issue with today's (5/20) mm-new branch.
>
> I've disabled PMD-sized THP in my system:
> [root]# cat /sys/kernel/mm/transparent_hugepage/enabled
> always madvise [never]
> [root]# cat /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/enabled
> always inherit madvise [never]
>
> And I tried calling madvise() with MADV_COLLAPSE for anonymous memory,
> and I can still see it collapsing to a PMD-sized THP.
Hi Baolin ! Thank you for your reply and willingness to test again :)

I didn't realize we were talking about madvise collapse-- this makes
sense now. I also figured out why I could "reproduce" it before. My
script was always enabling the THP settings in two places, and I only
commented out one to test this. But this time I was doing more manual
testing.

The original design of madvise_collapse ignores the sysfs and
collapses even if you have an order disabled. I believe this behavior
is wrong, but by design. I spent some time playing around with madvise
collapses with and w/o my changes. This is not a new thing, I
reproduced the issue in 6.11 (Fedora 41), and I think its been
possible since the inception of madvise collapse 3 years ago. I
noticed a similar behavior on one of my RFC since it was "breaking"
selftests, and the fix was to reincorporate this broken sysfs
behavior.

7d8faaf15545 ("mm/madvise: introduce MADV_COLLAPSE sync hugepage collapse")
"This call is independent of the system-wide THP sysfs settings, but
will fail for memory marked VM_NOHUGEPAGE."

The second condition holds true (and fails for VM_NOHUGEPAGE), but I
dont know if we actually want madvise_collapse to be independent of
the system-wide.

So I'll ask the authors
+David Rientjes +zokeefe@google.com
Was this brought up as a concern when this feature was first
introduced, was there any pushback, what was the outcome of the
discussion if so?
I can easily fix this and it would further simplify the code (by
removing the is_khugepaged and friends). As David H. has brought up in
other discussions around similar topics, never should mean never, is
this the only exception we should allow?

Thanks!
>
> > I can no longer reproduce this issue, that's why I posted... although
> > I should have followed up, and looked into what the original issue
> > was. Nothing really sticks out so perhaps something in mm-new was
> > broken and pulled out... not sure.
> >
> > It should now follow the expected behavior, which is that no mTHP
> > collapse occurs because if the PMD size is disabled so is khugepaged
> > collapse.
> >
> > Lmk if you are still experiencing this issue please.
> >
> > Cheers,
> > -- Nico
> >>
> >> [1]
> >> https://lore.kernel.org/all/83a66442-b7c7-42e7-af4e-fd211d8ed6f8@linux.alibaba.com/
> >>
>



  parent reply	other threads:[~2025-05-21 10:23 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-15  3:22 [PATCH v7 00/12] khugepaged: mTHP support Nico Pache
2025-05-15  3:22 ` [PATCH v7 01/12] khugepaged: rename hpage_collapse_* to khugepaged_* Nico Pache
2025-05-16 17:30   ` Liam R. Howlett
2025-06-29  6:48     ` Nico Pache
2025-05-15  3:22 ` [PATCH v7 02/12] introduce khugepaged_collapse_single_pmd to unify khugepaged and madvise_collapse Nico Pache
2025-05-15  5:50   ` Baolin Wang
2025-05-16 11:59     ` Nico Pache
2025-05-16 17:12   ` Liam R. Howlett
2025-07-02  0:00     ` Nico Pache
2025-05-15  3:22 ` [PATCH v7 03/12] khugepaged: generalize hugepage_vma_revalidate for mTHP support Nico Pache
2025-05-16 17:14   ` Liam R. Howlett
2025-06-29  6:52     ` Nico Pache
2025-05-23  6:55   ` Baolin Wang
2025-05-28  6:57     ` Dev Jain
2025-05-29  4:00     ` Nico Pache
2025-05-30  3:02       ` Baolin Wang
2025-05-15  3:22 ` [PATCH v7 04/12] khugepaged: generalize alloc_charge_folio() Nico Pache
2025-05-15  3:22 ` [PATCH v7 05/12] khugepaged: generalize __collapse_huge_page_* for mTHP support Nico Pache
2025-05-15  3:22 ` [PATCH v7 06/12] khugepaged: introduce khugepaged_scan_bitmap " Nico Pache
2025-05-16  3:20   ` Baolin Wang
2025-05-17  6:47     ` Nico Pache
2025-05-18  3:04       ` Liam R. Howlett
2025-05-20 10:09       ` Baolin Wang
2025-05-20 10:26         ` David Hildenbrand
2025-05-21  1:03           ` Baolin Wang
2025-05-21 10:23         ` Nico Pache [this message]
2025-05-22  9:39           ` Baolin Wang
2025-05-28  9:26             ` David Hildenbrand
2025-05-28 14:04               ` Baolin Wang
2025-05-29  4:02                 ` Nico Pache
2025-05-29  8:27                   ` Baolin Wang
2025-05-15  3:22 ` [PATCH v7 07/12] khugepaged: add " Nico Pache
2025-06-07  6:23   ` Dev Jain
2025-06-07 12:55     ` Nico Pache
2025-06-07 13:03     ` Nico Pache
2025-06-07 14:31       ` Dev Jain
2025-06-07 14:42         ` Dev Jain
2025-05-15  3:22 ` [PATCH v7 08/12] khugepaged: skip collapsing mTHP to smaller orders Nico Pache
2025-05-15  3:22 ` [PATCH v7 09/12] khugepaged: avoid unnecessary mTHP collapse attempts Nico Pache
2025-05-15  3:22 ` [PATCH v7 10/12] khugepaged: improve tracepoints for mTHP orders Nico Pache
2025-05-15  3:22 ` [PATCH v7 11/12] khugepaged: add per-order mTHP khugepaged stats Nico Pache
2025-05-15  3:22 ` [PATCH v7 12/12] Documentation: mm: update the admin guide for mTHP collapse Nico Pache
2025-05-15  4:40   ` Randy Dunlap
2025-06-07  6:44   ` Dev Jain
2025-06-07 12:57     ` Nico Pache
2025-06-07 14:34       ` Dev Jain
2025-06-08 19:50         ` Nico Pache
2025-06-09  3:06           ` Baolin Wang
2025-06-09  5:26             ` Dev Jain
2025-06-09  6:39               ` Baolin Wang
2025-06-09  5:56             ` Nico Pache
2025-05-28 12:31 ` [PATCH 1/2] mm: khugepaged: allow khugepaged to check all anonymous mTHP orders Baolin Wang
2025-05-28 12:31   ` [PATCH 2/2] mm: khugepaged: kick khugepaged for enabling none-PMD-sized mTHPs Baolin Wang
2025-05-28 12:39 ` [PATCH v7 00/12] khugepaged: mTHP support Baolin Wang
2025-05-29  3:52   ` Nico Pache
2025-06-16  3:51 ` Dev Jain
2025-06-16 15:51   ` Nico Pache
2025-06-16 16:35     ` Dev Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAA1CXcAWcahkxzsvK_bcWei6or_gKBjt+97dqhuSem8N7cBAQw@mail.gmail.com \
    --to=npache@redhat.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=catalin.marinas@arm.com \
    --cc=cl@gentwo.org \
    --cc=corbet@lwn.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@redhat.com \
    --cc=dev.jain@arm.com \
    --cc=hannes@cmpxchg.org \
    --cc=jack@suse.cz \
    --cc=jglisse@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mhocko@suse.com \
    --cc=peterx@redhat.com \
    --cc=raquini@redhat.com \
    --cc=rdunlap@infradead.org \
    --cc=rientjes@google.com \
    --cc=rostedt@goodmis.org \
    --cc=ryan.roberts@arm.com \
    --cc=sunnanyong@huawei.com \
    --cc=surenb@google.com \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=tiwai@suse.de \
    --cc=usamaarif642@gmail.com \
    --cc=vishal.moola@gmail.com \
    --cc=wangkefeng.wang@huawei.com \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --cc=yang@os.amperecomputing.com \
    --cc=ziy@nvidia.com \
    --cc=zokeefe@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).