[REGRESSION] nfs: Large amounts of GETATTR calls after file renaming on v5.10.241

[REGRESSION] nfs: Large amounts of GETATTR calls after file renaming on v5.10.241

[Ahmed, Aaron]

Hi,

We have had customers report a regression on kernels versions 5.10.241 and above in which file renaming causes large amounts of GETATTR calls to made due to inode revalidation. This regression was pinpointed via bisected to commit 7378c7adf31d ("NFS: Don't set NFS_INO_REVAL_PAGECACHE in the inode cache validity") which is a backport of 36a9346c2252 (“NFS: Don't set NFS_INO_REVAL_PAGECACHE in the inode cache validity”). 

We were able to reproduce It with this script:
REPRO_PATH=/mnt/efs/repro
do_read()
{
    for x in {1..50}
    do
        cat $1 > /dev/null
    done
    grep GETATTR /proc/self/mountstats
}

echo foo > $REPRO_PATH/bar
echo "After create, before read:"
grep GETATTR /proc/self/mountstats

echo "First read:"
do_read $REPRO_PATH/bar

echo "Sleeping 5s, reading again (should look the same):"
sleep 5
do_read $REPRO_PATH/bar

mv $REPRO_PATH/bar $REPRO_PATH/baz
echo "Moved file, reading again:"
do_read $REPRO_PATH/baz

echo "Immediately performing another set of reads:"
do_read $REPRO_PATH/baz

echo "Cleanup, removing test file"
rm $REPRO_PATH/baz
which performs a few read/writes. On kernels without the regression the number of GETATTR calls remains the same while on affected kernels the amount increases after reading renamed file. 

This original commit comes from a series of patches providing attribute revalidation updates [1].  However, many of these patches are missing in v.5.10.241+. Specifically, 13c0b082b6a9 (“NFS: Replace use of NFS_INO_REVAL_PAGECACHE when checking cache validity”) seems like a prerequisite patch and would help remedy the regression.

#regzbot introduced: 7378c7adf31d

Thanks,
Aaron Ahmed

[1]  https://lore.kernel.org/all/20210414134353.11860-1-trondmy@kernel.org/

Signed-off-by: Aaron Ahmed <aarnhahmd@amazon.com>

Re: [REGRESSION] nfs: Large amounts of GETATTR calls after file renaming on v5.10.241

[gregkh]

On Fri, Nov 21, 2025 at 06:56:31PM +0000, Ahmed, Aaron wrote:
> Hi,
> 
> We have had customers report a regression on kernels versions 5.10.241 and above in which file renaming causes large amounts of GETATTR calls to made due to inode revalidation. This regression was pinpointed via bisected to commit 7378c7adf31d ("NFS: Don't set NFS_INO_REVAL_PAGECACHE in the inode cache validity") which is a backport of 36a9346c2252 (“NFS: Don't set NFS_INO_REVAL_PAGECACHE in the inode cache validity”). 
> 
> We were able to reproduce It with this script:
> REPRO_PATH=/mnt/efs/repro
> do_read()
> {
>     for x in {1..50}
>     do
>         cat $1 > /dev/null
>     done
>     grep GETATTR /proc/self/mountstats
> }
> 
> echo foo > $REPRO_PATH/bar
> echo "After create, before read:"
> grep GETATTR /proc/self/mountstats
> 
> echo "First read:"
> do_read $REPRO_PATH/bar
> 
> echo "Sleeping 5s, reading again (should look the same):"
> sleep 5
> do_read $REPRO_PATH/bar
> 
> mv $REPRO_PATH/bar $REPRO_PATH/baz
> echo "Moved file, reading again:"
> do_read $REPRO_PATH/baz
> 
> echo "Immediately performing another set of reads:"
> do_read $REPRO_PATH/baz
> 
> echo "Cleanup, removing test file"
> rm $REPRO_PATH/baz
> which performs a few read/writes. On kernels without the regression the number of GETATTR calls remains the same while on affected kernels the amount increases after reading renamed file. 
> 
> This original commit comes from a series of patches providing attribute revalidation updates [1].  However, many of these patches are missing in v.5.10.241+. Specifically, 13c0b082b6a9 (“NFS: Replace use of NFS_INO_REVAL_PAGECACHE when checking cache validity”) seems like a prerequisite patch and would help remedy the regression.

Can you please send the needed backports to resolve this issue as you
can test and verify that this resolves the problem?

thanks,

greg k-h

Re: [REGRESSION] nfs: Large amounts of GETATTR calls after file renaming on v5.10.241

[Trond Myklebust]

On Sat, 2025-11-22 at 07:56 +0100, gregkh@linuxfoundation.org wrote:
> On Fri, Nov 21, 2025 at 06:56:31PM +0000, Ahmed, Aaron wrote:
> > Hi,
> > 
> > We have had customers report a regression on kernels versions
> > 5.10.241 and above in which file renaming causes large amounts of
> > GETATTR calls to made due to inode revalidation. This regression
> > was pinpointed via bisected to commit 7378c7adf31d ("NFS: Don't set
> > NFS_INO_REVAL_PAGECACHE in the inode cache validity") which is a
> > backport of 36a9346c2252 (“NFS: Don't set NFS_INO_REVAL_PAGECACHE
> > in the inode cache validity”). 
> > 
> > We were able to reproduce It with this script:
> > REPRO_PATH=/mnt/efs/repro
> > do_read()
> > {
> >     for x in {1..50}
> >     do
> >         cat $1 > /dev/null
> >     done
> >     grep GETATTR /proc/self/mountstats
> > }
> > 
> > echo foo > $REPRO_PATH/bar
> > echo "After create, before read:"
> > grep GETATTR /proc/self/mountstats
> > 
> > echo "First read:"
> > do_read $REPRO_PATH/bar
> > 
> > echo "Sleeping 5s, reading again (should look the same):"
> > sleep 5
> > do_read $REPRO_PATH/bar
> > 
> > mv $REPRO_PATH/bar $REPRO_PATH/baz
> > echo "Moved file, reading again:"
> > do_read $REPRO_PATH/baz
> > 
> > echo "Immediately performing another set of reads:"
> > do_read $REPRO_PATH/baz
> > 
> > echo "Cleanup, removing test file"
> > rm $REPRO_PATH/baz
> > which performs a few read/writes. On kernels without the regression
> > the number of GETATTR calls remains the same while on affected
> > kernels the amount increases after reading renamed file. 
> > 
> > This original commit comes from a series of patches providing
> > attribute revalidation updates [1].  However, many of these patches
> > are missing in v.5.10.241+. Specifically, 13c0b082b6a9 (“NFS:
> > Replace use of NFS_INO_REVAL_PAGECACHE when checking cache
> > validity”) seems like a prerequisite patch and would help remedy
> > the regression.
> 
> Can you please send the needed backports to resolve this issue as you
> can test and verify that this resolves the problem?
> 
> thanks,
> 
> greg k-h

Ah... If I'm correctly reading the changelog
in https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.241, it
looks as if commit 36a9346c2252 got pulled in by the stable patch
automation as a dependency for commit b01f21cacde9 ("NFS: Fix the
setting of capabilities when automounting a new filesystem").

I do agree with Aaron's assessment that patch does depend on the rest
of the series that was merged into Linux 5.13. It cannot be cherry-
picked on its own.

However what exactly was the dependency that caused it to be pulled
into 5.10.241 in the first place? Was that logged anywhere?
I just checked out v5.10.241 and applied "git revert --signoff
36a9346c2252", and that appears to work just fine, and I don't see any
other obvious code dependency there, so I'm curious about what
happened.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [REGRESSION] nfs: Large amounts of GETATTR calls after file renaming on v5.10.241

[gregkh]

On Sat, Nov 22, 2025 at 10:43:29AM -0500, Trond Myklebust wrote:
> On Sat, 2025-11-22 at 07:56 +0100, gregkh@linuxfoundation.org wrote:
> > On Fri, Nov 21, 2025 at 06:56:31PM +0000, Ahmed, Aaron wrote:
> > > Hi,
> > > 
> > > We have had customers report a regression on kernels versions
> > > 5.10.241 and above in which file renaming causes large amounts of
> > > GETATTR calls to made due to inode revalidation. This regression
> > > was pinpointed via bisected to commit 7378c7adf31d ("NFS: Don't set
> > > NFS_INO_REVAL_PAGECACHE in the inode cache validity") which is a
> > > backport of 36a9346c2252 (“NFS: Don't set NFS_INO_REVAL_PAGECACHE
> > > in the inode cache validity”). 
> > > 
> > > We were able to reproduce It with this script:
> > > REPRO_PATH=/mnt/efs/repro
> > > do_read()
> > > {
> > >     for x in {1..50}
> > >     do
> > >         cat $1 > /dev/null
> > >     done
> > >     grep GETATTR /proc/self/mountstats
> > > }
> > > 
> > > echo foo > $REPRO_PATH/bar
> > > echo "After create, before read:"
> > > grep GETATTR /proc/self/mountstats
> > > 
> > > echo "First read:"
> > > do_read $REPRO_PATH/bar
> > > 
> > > echo "Sleeping 5s, reading again (should look the same):"
> > > sleep 5
> > > do_read $REPRO_PATH/bar
> > > 
> > > mv $REPRO_PATH/bar $REPRO_PATH/baz
> > > echo "Moved file, reading again:"
> > > do_read $REPRO_PATH/baz
> > > 
> > > echo "Immediately performing another set of reads:"
> > > do_read $REPRO_PATH/baz
> > > 
> > > echo "Cleanup, removing test file"
> > > rm $REPRO_PATH/baz
> > > which performs a few read/writes. On kernels without the regression
> > > the number of GETATTR calls remains the same while on affected
> > > kernels the amount increases after reading renamed file. 
> > > 
> > > This original commit comes from a series of patches providing
> > > attribute revalidation updates [1].  However, many of these patches
> > > are missing in v.5.10.241+. Specifically, 13c0b082b6a9 (“NFS:
> > > Replace use of NFS_INO_REVAL_PAGECACHE when checking cache
> > > validity”) seems like a prerequisite patch and would help remedy
> > > the regression.
> > 
> > Can you please send the needed backports to resolve this issue as you
> > can test and verify that this resolves the problem?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Ah... If I'm correctly reading the changelog
> in https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.241, it
> looks as if commit 36a9346c2252 got pulled in by the stable patch
> automation as a dependency for commit b01f21cacde9 ("NFS: Fix the
> setting of capabilities when automounting a new filesystem").
> 
> I do agree with Aaron's assessment that patch does depend on the rest
> of the series that was merged into Linux 5.13. It cannot be cherry-
> picked on its own.
> 
> However what exactly was the dependency that caused it to be pulled
> into 5.10.241 in the first place? Was that logged anywhere?
> I just checked out v5.10.241 and applied "git revert --signoff
> 36a9346c2252", and that appears to work just fine, and I don't see any
> other obvious code dependency there, so I'm curious about what
> happened.

As the commit says:
	Stable-dep-of: b01f21cacde9 ("NFS: Fix the setting of capabilities when automounting a new filesystem")

Re: [REGRESSION] nfs: Large amounts of GETATTR calls after file renaming on v5.10.241

[Trond Myklebust]

On Sun, 2025-11-23 at 07:23 +0100, gregkh@linuxfoundation.org wrote:
> On Sat, Nov 22, 2025 at 10:43:29AM -0500, Trond Myklebust wrote:
> > On Sat, 2025-11-22 at 07:56 +0100,
> > gregkh@linuxfoundation.org wrote:
> > > On Fri, Nov 21, 2025 at 06:56:31PM +0000, Ahmed, Aaron wrote:
> > > > Hi,
> > > > 
> > > > We have had customers report a regression on kernels versions
> > > > 5.10.241 and above in which file renaming causes large amounts
> > > > of
> > > > GETATTR calls to made due to inode revalidation. This
> > > > regression
> > > > was pinpointed via bisected to commit 7378c7adf31d ("NFS: Don't
> > > > set
> > > > NFS_INO_REVAL_PAGECACHE in the inode cache validity") which is
> > > > a
> > > > backport of 36a9346c2252 (“NFS: Don't set
> > > > NFS_INO_REVAL_PAGECACHE
> > > > in the inode cache validity”). 
> > > > 
> > > > We were able to reproduce It with this script:
> > > > REPRO_PATH=/mnt/efs/repro
> > > > do_read()
> > > > {
> > > >     for x in {1..50}
> > > >     do
> > > >         cat $1 > /dev/null
> > > >     done
> > > >     grep GETATTR /proc/self/mountstats
> > > > }
> > > > 
> > > > echo foo > $REPRO_PATH/bar
> > > > echo "After create, before read:"
> > > > grep GETATTR /proc/self/mountstats
> > > > 
> > > > echo "First read:"
> > > > do_read $REPRO_PATH/bar
> > > > 
> > > > echo "Sleeping 5s, reading again (should look the same):"
> > > > sleep 5
> > > > do_read $REPRO_PATH/bar
> > > > 
> > > > mv $REPRO_PATH/bar $REPRO_PATH/baz
> > > > echo "Moved file, reading again:"
> > > > do_read $REPRO_PATH/baz
> > > > 
> > > > echo "Immediately performing another set of reads:"
> > > > do_read $REPRO_PATH/baz
> > > > 
> > > > echo "Cleanup, removing test file"
> > > > rm $REPRO_PATH/baz
> > > > which performs a few read/writes. On kernels without the
> > > > regression
> > > > the number of GETATTR calls remains the same while on affected
> > > > kernels the amount increases after reading renamed file. 
> > > > 
> > > > This original commit comes from a series of patches providing
> > > > attribute revalidation updates [1].  However, many of these
> > > > patches
> > > > are missing in v.5.10.241+. Specifically, 13c0b082b6a9 (“NFS:
> > > > Replace use of NFS_INO_REVAL_PAGECACHE when checking cache
> > > > validity”) seems like a prerequisite patch and would help
> > > > remedy
> > > > the regression.
> > > 
> > > Can you please send the needed backports to resolve this issue as
> > > you
> > > can test and verify that this resolves the problem?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > Ah... If I'm correctly reading the changelog
> > in 
> > https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fc
> > dn.kernel.org%2Fpub%2Flinux%2Fkernel%2Fv5.x%2FChangeLog-
> > 5.10.241&data=05%7C02%7Ctrondmy%40hammerspace.com%7Cd99de46ea1054d1
> > a48c408de2a58f676%7C0d4fed5c3a7046fe9430ece41741f59e%7C0%7C0%7C6389
> > 94758780825772%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYi
> > OiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C
> > 0%7C%7C%7C&sdata=EbT6VZycJOQ7ktWkCq5jDYG9NdhP7vlkiK%2FuK9nIp9k%3D&r
> > eserved=0, it
> > looks as if commit 36a9346c2252 got pulled in by the stable patch
> > automation as a dependency for commit b01f21cacde9 ("NFS: Fix the
> > setting of capabilities when automounting a new filesystem").
> > 
> > I do agree with Aaron's assessment that patch does depend on the
> > rest
> > of the series that was merged into Linux 5.13. It cannot be cherry-
> > picked on its own.
> > 
> > However what exactly was the dependency that caused it to be pulled
> > into 5.10.241 in the first place? Was that logged anywhere?
> > I just checked out v5.10.241 and applied "git revert --signoff
> > 36a9346c2252", and that appears to work just fine, and I don't see
> > any
> > other obvious code dependency there, so I'm curious about what
> > happened.
> 
> As the commit says:
> 	Stable-dep-of: b01f21cacde9 ("NFS: Fix the setting of
> capabilities when automounting a new filesystem")
> 
> 
If that's all we have, then I suggest just applying the following
revert.

8<------------------------------------------------------------------
From 3ae48f4eccfe9dcfeb9b3ff3670cadaed7b35c9e Mon Sep 17 00:00:00 2001
Message-ID: <3ae48f4eccfe9dcfeb9b3ff3670cadaed7b35c9e.1763918082.git.trond.myklebust@hammerspace.com>
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Date: Sat, 22 Nov 2025 10:28:09 -0500
Subject: [PATCH] Revert "NFS: Don't set NFS_INO_REVAL_PAGECACHE in the inode
 cache validity"

This reverts commit 36a9346c225270262d9f34e66c91aa1723fa903f.

The above commit was incorrectly labelled as a dependency for commit
b01f21cacde9 ("NFS: Fix the setting of capabilities when automounting a
new filesystem")
A revert is needed, since the incorrectly applied commit depends upon a
series of other patches that were merged into Linux 5.13, but have not
been applied to the 5.10 stable series.

Reported-by: "Ahmed, Aaron" <aarnahmd@amazon.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/inode.c    | 6 ++++--
 fs/nfs/nfs4proc.c | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index da8d727eb09d..3e3114a9d193 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -217,12 +217,11 @@ static void nfs_set_cache_invalid(struct inode *inode, unsigned long flags)
 			flags &= ~NFS_INO_INVALID_OTHER;
 		flags &= ~(NFS_INO_INVALID_CHANGE
 				| NFS_INO_INVALID_SIZE
+				| NFS_INO_REVAL_PAGECACHE
 				| NFS_INO_INVALID_XATTR);
 	} else if (flags & NFS_INO_REVAL_PAGECACHE)
 		flags |= NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE;
 
-	flags &= ~NFS_INO_REVAL_PAGECACHE;
-
 	if (!nfs_has_xattr_cache(nfsi))
 		flags &= ~NFS_INO_INVALID_XATTR;
 	if (inode->i_mapping->nrpages == 0)
@@ -1901,6 +1900,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
 			| NFS_INO_INVALID_ATIME
 			| NFS_INO_REVAL_FORCED
+			| NFS_INO_REVAL_PAGECACHE
 			| NFS_INO_INVALID_BLOCKS);
 
 	/* Do atomic weak cache consistency updates */
@@ -1942,6 +1942,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	} else {
 		nfsi->cache_validity |= save_cache_validity &
 				(NFS_INO_INVALID_CHANGE
+				| NFS_INO_REVAL_PAGECACHE
 				| NFS_INO_REVAL_FORCED);
 		cache_revalidated = false;
 	}
@@ -1987,6 +1988,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	} else {
 		nfsi->cache_validity |= save_cache_validity &
 				(NFS_INO_INVALID_SIZE
+				| NFS_INO_REVAL_PAGECACHE
 				| NFS_INO_REVAL_FORCED);
 		cache_revalidated = false;
 	}
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 973b708ff332..2c33436bb27e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1210,6 +1210,7 @@ nfs4_update_changeattr_locked(struct inode *inode,
 		| cache_validity;
 
 	if (cinfo->atomic && cinfo->before == inode_peek_iversion_raw(inode)) {
+		nfsi->cache_validity &= ~NFS_INO_REVAL_PAGECACHE;
 		nfsi->attrtimeo_timestamp = jiffies;
 	} else {
 		if (S_ISDIR(inode->i_mode)) {
-- 
2.51.1