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

[David Hildenbrand (Arm)]

On 4/21/26 08:27, Baolin Wang wrote:
> 
> 
> On 4/21/26 3:00 AM, David Hildenbrand (Arm) wrote:
>> On 4/17/26 14:45, Baolin Wang wrote:
>>>
>>>
>>>
>>> Indeed. Good point.
>>>
>>>
>>> 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.
>>
>> Okay, so you'll send a patch to just set mapping_set_large_folios()
>> unconditionally?
> 
> I'm still hesitating on this. If we set mapping_set_large_folios()
> unconditionally, we need to re-fix the performance regression that was
> addressed by commit 5a90c155defa.

Just so I can follow: where is the test for large folios that we would
unlock large folios and cause a regression?

> 
> But it's hard for me to convince myself to add a new flag similar to
> IOCB_NO_LARGE_CHUNK for this hack (like the patch in [1] does).
> 
>>> [1] https://lore.kernel.org/all/20240914140613.2334139-1-
>>> wangkefeng.wang@xxxxxxxxxx/
>>
>> Is that really required? Which call path would be the problematic bit
>> with the above?
>>
>> I'd say, we'd check in the large folio allocation code whether ->huge is
>> set to never instead?
> 
> Yes, this is exactly our current logic. When allocating large folios,
> we'll check the ->huge setting in shmem_huge_global_enabled(), which
> means large folio allocations always respect the ->huge setting.

Makes sense.

> 
> But as I mentioned earlier, the ->huge setting cannot keep the
> mapping_set_large_folios() setting consistent across all mappings in the
> entire tmpfs mount. My concern is that under the same tmpfs mount, after
> remount, we might end up with some mappings supporting large folios
> (calling mapping_set_large_folios()) while others don't.

If we at least always set mapping_set_large_folios(), then there is no
inconsistency in that regard :)

> 
> However, I got some insights from Documentation/admin-guide/mm/
> transhuge.rst. Does this mean that after remount, whether the mappings
> of existing files support large folios should remain unchanged?

That's the current behavior, right?

> 
> “
> ``mount -o remount,huge= /mountpoint`` works fine after mount:
> remounting ``huge=never`` will not attempt to break up huge pages at
> all, just stop more from being allocated.
> ”
> 
> Do you think this makes sense?

I suspect that matches existing behavior, so it should be fine.

-- 
Cheers,

David