[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a3910f60-6e2e-4ede-b3f3-47d8dfe9f23b@gmail.com>
Date: Thu, 4 Jul 2024 22:46:29 +0800
From: Bang Li <libang.linux@...il.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>,
Ryan Roberts <ryan.roberts@....com>, akpm@...ux-foundation.org,
hughd@...gle.com
Cc: willy@...radead.org, david@...hat.com, wangkefeng.wang@...wei.com,
ying.huang@...el.com, 21cnbao@...il.com, shy828301@...il.com,
ziy@...dia.com, ioworker0@...il.com, da.gomez@...sung.com,
p.raghav@...sung.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 4/6] mm: shmem: add mTHP support for anonymous shmem
Hi Bao lin,
On 2024/7/4 19:15, Baolin Wang wrote:
>
>>> +
>>> + /*
>>> + * Only allow inherit orders if the top-level value is 'force',
>>> which
>>> + * means non-PMD sized THP can not override 'huge' mount option
>>> now.
>>> + */
>>> + if (shmem_huge == SHMEM_HUGE_FORCE)
>>> + return READ_ONCE(huge_shmem_orders_inherit);
>>
>> I vaguely recall that we originally discussed that trying to set
>> 'force' on the
>> top level control while any per-size controls were set to 'inherit'
>> would be an
>> error, and trying to set 'force' on any per-size control except the
>> PMD-size
>> would be an error?
>
> Right.
>
>> I don't really understand this logic. Shouldn't we just be looking at
>> the
>> per-size control settings (or the top-level control as a proxy for every
>> per-size control that has 'inherit' set)?
>
> ‘force’ will apply the huge orders for anon shmem and tmpfs, so now we
> only allow pmd-mapped THP to be forced. We should not look at per-size
> control settings for tmpfs now (mTHP for tmpfs will be discussed in
> future).
>
>>
>> Then for tmpfs, which doesn't support non-PMD-sizes yet, we just
>> always use the
>> PMD-size control for decisions.
>>
>> I'm also really struggling with the concept of shmem_is_huge()
>> existing along
>> side shmem_allowable_huge_orders(). Surely this needs to all be
>> refactored into
>> shmem_allowable_huge_orders()?
>
> I understood. But now they serve different purposes: shmem_is_huge()
> will be used to check the huge orders for the top level, for *tmpfs*
> and anon shmem; whereas shmem_allowable_huge_orders() will only be
> used to check the per-size huge orders for anon shmem (excluding tmpfs
> now). However, as I plan to add mTHP support for tmpfs, I think we can
> perform some cleanups.
Please count me in, I'd be happy to contribute to the cleanup and
enhancement
process if I can.
Thanks,
Bang
Powered by blists - more mailing lists