Re: [PATCH v3] mm: shmem: always support large folios for internal shmem mount

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 4/17/26 5:52 PM, David Hildenbrand (Arm) wrote:
> On 4/17/26 11:27, Baolin Wang wrote:
>>
>>
>> On 4/17/26 5:21 PM, David Hildenbrand (Arm) wrote:
>>> On 4/17/26 05:25, Baolin Wang wrote:
>>>> Currently, when shmem mounts are initialized, they only use 'sbinfo-
>>>>> huge' to
>>>> determine whether the shmem mount supports large folios. However, for
>>>> anonymous
>>>> shmem, whether it supports large folios can be dynamically configured
>>>> via sysfs
>>>> interfaces, so setting or not setting mapping_set_large_folios()
>>>> during initialization
>>>> cannot accurately reflect whether anonymous shmem actually supports
>>>> large folios,
>>>> which has already caused some confusion[1].
>>>>
>>>> As discussed with David[2], for anonymous shmem we can treat it as
>>>> always potentially
>>>> having large folios. Therefore, always support large folios for the
>>>> internal shmem
>>>> mount (e.g., anonymous shmem), and which large order allocations are
>>>> allowed can be
>>>> configured dynamically via the 'shmem_enabled' interfaces.
>>>>
>>>> [1] https://lore.kernel.org/all/
>>>> ec927492-4577-4192-8fad-85eb1bb43121@linux.alibaba.com/
>>>> [2] https://lore.kernel.org/
>>>> all/875dc63b-0cd2-49e5-8b0d-3fb062789813@kernel.org/
>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>>> ---
>>>> Changes from v2:
>>>>    - Always support large folios for internal shmem mount, per David.
>>>> Changes from v1:
>>>>    - Update the comments and commit message, per Lance.
>>>> ---
>>>>    mm/shmem.c | 13 +++++++++++--
>>>>    1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mm/shmem.c b/mm/shmem.c
>>>> index 4ecefe02881d..769ef37b1ea9 100644
>>>> --- a/mm/shmem.c
>>>> +++ b/mm/shmem.c
>>>> @@ -3088,8 +3088,17 @@ static struct inode *__shmem_get_inode(struct
>>>> mnt_idmap *idmap,
>>>>        if (sbinfo->noswap)
>>>>            mapping_set_unevictable(inode->i_mapping);
>>>>    -    /* Don't consider 'deny' for emergencies and 'force' for
>>>> testing */
>>>> -    if (sbinfo->huge)
>>>> +    /*
>>>> +     * Always support large folios for the internal shmem mount (e.g.,
>>>> +     * anonymous shmem), and which large order allocations are allowed
>>>> +     * can be configured dynamically via the 'shmem_enabled'
>>>> interfaces.
>>>> +     *
>>>> +     * For tmpfs, honour the 'huge=' mount option to determine whether
>>>> +     * large folios are supported.
>>>> +     *
>>>> +     * Note: don't consider 'deny' for emergencies and 'force' for
>>>> testing.
>>>> +     */
>>>> +    if (sbinfo->huge || (sb->s_flags & SB_KERNMOUNT))
>>>>            mapping_set_large_folios(inode->i_mapping);
>>>
>>> Two questions from a non-fs person about the semantics here:
>>>
>>> a) Can sbinfo->huge be triggered later, for example, through a remount
>>> (staring at shmem_reconfigure())
>>
>> For tmpfs, yes.
> 
> So, we could pass this check here, not setting
> mapping_set_large_folios(), but later someone toggles it and we missed
> to set mapping_set_large_folios()?

Indeed. Good point.

> 
> Or would we always go through another __shmem_get_inode() after a remount?

Not really. There could be files created before remount whose mappings 
don't support large folios (with 'huge=never' option), while files 
created after remount will have mappings that support large folios (if 
remounted with 'huge=always' option).

It looks like the previous commit 5a90c155defa was also problematic. The 
huge mount option has introduced a lot of tricky issues:(

Now I think Zi's previous suggestion should be able to clean up this 
mess? That is, calling mapping_set_large_folios() unconditionally for 
all shmem mounts, and revisiting Kefeng's first version to fix the 
performance issue.

[1] 
https://lore.kernel.org/all/20240914140613.2334139-1-wangkefeng.wang@huawei.com/