[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8de73035-8645-4c53-80a5-de63128d388e@redhat.com>
Date: Wed, 25 Jun 2025 10:53:40 +0200
From: David Hildenbrand <david@...hat.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Hugh Dickins <hughd@...gle.com>,
Baolin Wang <baolin.wang@...ux.alibaba.com>, akpm@...ux-foundation.org,
ziy@...dia.com, Liam.Howlett@...cle.com, npache@...hat.com,
ryan.roberts@....com, dev.jain@....com, baohua@...nel.org,
zokeefe@...gle.com, shy828301@...il.com, usamaarif642@...il.com,
linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 0/2] fix MADV_COLLAPSE issue if THP settings are
disabled
On 25.06.25 10:37, Lorenzo Stoakes wrote:
> On Wed, Jun 25, 2025 at 10:24:53AM +0200, David Hildenbrand wrote:
>> On 25.06.25 10:12, Lorenzo Stoakes wrote:
>>> On Wed, Jun 25, 2025 at 08:55:28AM +0100, Lorenzo Stoakes wrote:
>>>> I suppose the least awful way of addressing Baolin's concerns re: mTHP
>>>> while simultaneosly keeping existing semantics is:
>>>>
>>>> 1. Introduce deny to mean what never should have meant.
>>>
>>> To fix Baolin's issue btw we'd have to add 'deny' to both 'global' settings
>>> _and_ each page size setting.
>>>
>>> Because otherwise we'd end up in a weird case where say:
>>>
>>> global 'deny'
>>>
>>> 2 MiB 'never'
>>> 64 KiB 'inherit'
>>>
>>> And err... get 2 MiB THP pages from MADV_COLLAPSE :)
>>>
>>> Or:
>>>
>>> global 'deny'
>>>
>>> 2 MiB 'never'
>>> 64 KiB 'always'
>>>
>>> Or:
>>>
>>> global 'never'
>>>
>>> 2 MiB 'never'
>>> 64 KiB 'always'
>>>
>>> Or:
>>>
>>> global 'never'
>>>
>>> 2 MiB 'madvise'
>>> 64 KiB 'always'
>>>
>>> All doing the same. Not very clear is it?
>>>
>>> We have sowed the seeds of something terrible here, truly.
>>
>> Fully agreed. "Deny" is nasty. Maybe if we really need a way to disable
>> "madv_collapse", it should be done differently, not using this toggle here.
>
> Yeah maybe the best way is to just have another tunable for this?
>
> /sys/kernel/mm/transparent_hugepage/disable_collapse perhaps?
>
> What do you think Hugh, Baolin?
Probably, for debugging purposes, a kernel cmdline option would work as
well. Something we can just easily change later.
>
>>
>> Regarding MADV_COLLAPSE, I strongly assume that we should not change it to
>> collapse smaller mTHPs as part of the khugepaged mTHP work. For now, it will
>> simply always collapse to PMD THPs.
>
> Yeah thinking about it maybe this is the best way. And we can then update
> the man page to make this ABUNDANTLY clear (am happy to do this).
>
> This keeps things simple.
>
> (One side note on PMD-sized MADV_COLLAPSE - this is basically completely
> useless for 64 KB page size arm64 systems where PMD's are 512 MB :)
>
> Thoughts Baolin?
>
>>
>> Once we want to support other sizes, likely MADV_COLLAPSE users want to have
>> better control over which size to use, at which point it all gets nasty.
>
> madvise2() this time with extra parameters? ;)
>
> I sort of wish we had added a flags parameter there.
>
> But lacking a time machine... :)
Reminds me of how MAP_HUGETLB squeezed in the sizes by stealing plenty
of bits ... :)
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists