Re: [PATCH] Revert "tmpfs: don't enable large folios if not supported"

Re: [PATCH] Revert "tmpfs: don't enable large folios if not supported"

[Lorenzo Stoakes]

On Thu, Apr 23, 2026 at 09:41:42AM +0800, Baolin Wang wrote:
> This reverts commit 5a90c155defa684f3a21f68c3f8e40c056e6114c.
>
> 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].
>
> Moreover, for tmpfs mounts, relying on 'sbinfo->huge' cannot keep the mapping_set_large_folios()
> setting consistent across all mappings in the entire tmpfs mount. In other words,
> 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.
>
> After some investigation, I found that the write performance regression addressed
> by commit 5a90c155defa has already been fixed by the following commit 665575cff098b
> ("filemap: move prefaulting out of hot write path"). See the following test data:
>
> Base:
> dd if=/dev/zero of=/mnt/tmpfs/test bs=400K count=10485 (3.2 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/test bs=800K count=5242 (3.2 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/test bs=1600K count=2621 (3.1 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/test bs=2200K count=1906 (3.0 GB/s )
> dd if=/dev/zero of=/mnt/tmpfs/test bs=3000K count=1398 (3.0 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/test bs=4500K count=932 (3.1 GB/s)
>
> Base + revert 5a90c155defa:
> dd if=/dev/zero of=/mnt/tmpfs/test bs=400K count=10485 (3.3 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/test bs=800K count=5242 (3.3 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/test bs=1600K count=2621 (3.2 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/test bs=2200K count=1906 (3.1 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/testbs=3000K count=1398 (3.0 GB/s)
> dd if=/dev/zero of=/mnt/tmpfs/test bs=4500K count=932 (3.1 GB/s)
>
> The data is basically consistent with minor fluctuation noise. So we can now
> safely revert commit 5a90c155defa to set mapping_set_large_folios() for all
> shmem mounts unconditionally.
>
> [1] https://lore.kernel.org/all/ec927492-4577-4192-8fad-85eb1bb43121@linux.alibaba.com/
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Acked-by: Lorenzo Stoakes <ljs@kernel.org>

As David asked, should we have a Fixes tag? What about cc: stable?

Cheers, Lorenzo

> ---
> Note: for more investigation and test data, see:
> https://lore.kernel.org/all/116df9f9-4db7-40d4-a4a4-30a87c0feffa@linux.alibaba.com/
> Thanks Kefeng for confirming the performance issue.
> ---
>  mm/shmem.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 4ecefe02881d..dafbea53b22d 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3087,10 +3087,7 @@ static struct inode *__shmem_get_inode(struct mnt_idmap *idmap,
>  	cache_no_acl(inode);
>  	if (sbinfo->noswap)
>  		mapping_set_unevictable(inode->i_mapping);
> -
> -	/* Don't consider 'deny' for emergencies and 'force' for testing */
> -	if (sbinfo->huge)
> -		mapping_set_large_folios(inode->i_mapping);
> +	mapping_set_large_folios(inode->i_mapping);
>
>  	switch (mode & S_IFMT) {
>  	default:
> --
> 2.47.3
>

Re: [PATCH] Revert "tmpfs: don't enable large folios if not supported"

[Lorenzo Stoakes]

On Tue, May 05, 2026 at 11:46:11AM +0100, Lorenzo Stoakes wrote:
> Acked-by: Lorenzo Stoakes <ljs@kernel.org>
>
> As David asked, should we have a Fixes tag? What about cc: stable?

Oh he mentioned link :P yes that too ;)

>
> Cheers, Lorenzo
>

Re: [PATCH] Revert "tmpfs: don't enable large folios if not supported"

[Baolin Wang]

On 5/5/26 6:47 PM, Lorenzo Stoakes wrote:
> On Tue, May 05, 2026 at 11:46:11AM +0100, Lorenzo Stoakes wrote:
>> Acked-by: Lorenzo Stoakes <ljs@kernel.org>
>>
>> As David asked, should we have a Fixes tag? What about cc: stable?

Sure. But this fix tag depends on the commit 665575cff098b, so the fixes 
tag should be:

Fixes: 5a90c155defa ("tmpfs: don't enable large folios if not supported")
Depends-on: 665575cff098b ("filemap: move prefaulting out of hot write 
path").

I don't think we need to cc stable, as it doesn't appear to cause any 
serious issues currently.

Re: [PATCH] Revert "tmpfs: don't enable large folios if not supported"

[Lorenzo Stoakes]

On Wed, May 06, 2026 at 04:58:20PM +0800, Baolin Wang wrote:
>
>
> On 5/5/26 6:47 PM, Lorenzo Stoakes wrote:
> > On Tue, May 05, 2026 at 11:46:11AM +0100, Lorenzo Stoakes wrote:
> > > Acked-by: Lorenzo Stoakes <ljs@kernel.org>
> > >
> > > As David asked, should we have a Fixes tag? What about cc: stable?
>
> Sure. But this fix tag depends on the commit 665575cff098b, so the fixes tag
> should be:
>
> Fixes: 5a90c155defa ("tmpfs: don't enable large folios if not supported")
> Depends-on: 665575cff098b ("filemap: move prefaulting out of hot write
> path").

Oh is Depends-on a tag? Wasn't aware of it :>)

>
> I don't think we need to cc stable, as it doesn't appear to cause any
> serious issues currently.

Ack ok!

Cheers, Lorenzo

Re: [PATCH] Revert "tmpfs: don't enable large folios if not supported"

[Andrew Morton]

On Mon, 11 May 2026 13:03:04 +0100 Lorenzo Stoakes <ljs@kernel.org> wrote:

> On Wed, May 06, 2026 at 04:58:20PM +0800, Baolin Wang wrote:
> >
> >
> > On 5/5/26 6:47 PM, Lorenzo Stoakes wrote:
> > > On Tue, May 05, 2026 at 11:46:11AM +0100, Lorenzo Stoakes wrote:
> > > > Acked-by: Lorenzo Stoakes <ljs@kernel.org>
> > > >
> > > > As David asked, should we have a Fixes tag? What about cc: stable?
> >
> > Sure. But this fix tag depends on the commit 665575cff098b, so the fixes tag
> > should be:
> >
> > Fixes: 5a90c155defa ("tmpfs: don't enable large folios if not supported")
> > Depends-on: 665575cff098b ("filemap: move prefaulting out of hot write
> > path").
> 
> Oh is Depends-on a tag? Wasn't aware of it :>)

It is not.

hp2:/usr/src/25> grep -ri Depends-on Documentation | wc -l 
0

But it is!

hp2:/usr/src/25> grep -ri Depends-on ../gitlog | wc -l    
156

I actually removed it because a) various tools whine at me and b) we
didn't cc:stable anyway.

Please let's not add new metadata without having told others what that
metadata means?

"Fixes:" should be sufficient.  I interpret Fixes: as "apply this to any
kernel which contains <that>".  Is more needed?

Re: [PATCH] Revert "tmpfs: don't enable large folios if not supported"

[Baolin Wang]

On 5/12/26 6:01 AM, Andrew Morton wrote:
> On Mon, 11 May 2026 13:03:04 +0100 Lorenzo Stoakes <ljs@kernel.org> wrote:
> 
>> On Wed, May 06, 2026 at 04:58:20PM +0800, Baolin Wang wrote:
>>>
>>>
>>> On 5/5/26 6:47 PM, Lorenzo Stoakes wrote:
>>>> On Tue, May 05, 2026 at 11:46:11AM +0100, Lorenzo Stoakes wrote:
>>>>> Acked-by: Lorenzo Stoakes <ljs@kernel.org>
>>>>>
>>>>> As David asked, should we have a Fixes tag? What about cc: stable?
>>>
>>> Sure. But this fix tag depends on the commit 665575cff098b, so the fixes tag
>>> should be:
>>>
>>> Fixes: 5a90c155defa ("tmpfs: don't enable large folios if not supported")
>>> Depends-on: 665575cff098b ("filemap: move prefaulting out of hot write
>>> path").
>>
>> Oh is Depends-on a tag? Wasn't aware of it :>)
> 
> It is not.
> 
> hp2:/usr/src/25> grep -ri Depends-on Documentation | wc -l
> 0
> 
> But it is!
> 
> hp2:/usr/src/25> grep -ri Depends-on ../gitlog | wc -l
> 156
> 
> I actually removed it because a) various tools whine at me and b) we
> didn't cc:stable anyway.

Okay, I remember I've seen that tag :)

> 
> Please let's not add new metadata without having told others what that
> metadata means?
> 
> "Fixes:" should be sufficient.  I interpret Fixes: as "apply this to any
> kernel which contains <that>".  Is more needed?

My concern is that if someone backports this patch to an older kernel 
version without also backporting commit 665575cff098b, it would cause a 
performance regression.

Is there a good way to mark this kind of dependency on another patch? 
But I've already described this potential dependency in the commit 
message, so I'm fine with just adding the "Fixes:" tag.

Re: [PATCH] Revert "tmpfs: don't enable large folios if not supported"

[Andrew Morton]

On Tue, 12 May 2026 08:55:05 +0800 Baolin Wang <baolin.wang@linux.alibaba.com> wrote:

> > 
> > Please let's not add new metadata without having told others what that
> > metadata means?
> > 
> > "Fixes:" should be sufficient.  I interpret Fixes: as "apply this to any
> > kernel which contains <that>".  Is more needed?
> 
> My concern is that if someone backports this patch to an older kernel 
> version without also backporting commit 665575cff098b, it would cause a 
> performance regression.
> 
> Is there a good way to mark this kind of dependency on another patch? 

Not that I know of.  Has someone sent 665575cff098b to -stable
maintainers and asked them to treat it as Fixes: 5a90c155defa?

> But I've already described this potential dependency in the commit 
> message, so I'm fine with just adding the "Fixes:" tag.

Yeah, I guess the supporting text will help clear confusion.

But you're right - our tooling should be able to express such things.