From: Aurelien Aptel aaptel@suse.com
Assuming - //HOST/a is mounted on /mnt - //HOST/b is mounted on /mnt/b
On a slow connection, running 'df' and killing it while it's processing /mnt/b can make cifs_get_inode_info() returns -ERESTARTSYS.
This triggers the following chain of events: => the dentry revalidation fail => dentry is put and released => superblock associated with the dentry is put => /mnt/b is unmounted
This patch makes cifs_d_revalidate() return the error instead of 0 (invalid) when cifs_revalidate_dentry() fails, except for ENOENT (file deleted) and ESTALE (file recreated).
Signed-off-by: Aurelien Aptel aaptel@suse.com Suggested-by: Shyam Prasad N nspmangalore@gmail.com CC: stable@vger.kernel.org
--- fs/cifs/dir.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 68900f1629bff..97ac363b5df16 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -737,6 +737,7 @@ static int cifs_d_revalidate(struct dentry *direntry, unsigned int flags) { struct inode *inode; + int rc;
if (flags & LOOKUP_RCU) return -ECHILD; @@ -746,8 +747,25 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags) if ((flags & LOOKUP_REVAL) && !CIFS_CACHE_READ(CIFS_I(inode))) CIFS_I(inode)->time = 0; /* force reval */
- if (cifs_revalidate_dentry(direntry)) - return 0; + rc = cifs_revalidate_dentry(direntry); + if (rc) { + cifs_dbg(FYI, "cifs_revalidate_dentry failed with rc=%d", rc); + switch (rc) { + case -ENOENT: + case -ESTALE: + /* + * Those errors mean the dentry is invalid + * (file was deleted or recreated) + */ + return 0; + default: + /* + * Otherwise some unexpected error happened + * report it as-is to VFS layer + */ + return rc; + } + } else { /* * If the inode wasn't known to be a dfs entry when
Looks good to me. Maybe change the FYI in the cifs_dbg line above to VFS?
On Fri, Feb 5, 2021 at 6:42 AM Aurélien Aptel aaptel@suse.com wrote:
From: Aurelien Aptel aaptel@suse.com
Assuming
- //HOST/a is mounted on /mnt
- //HOST/b is mounted on /mnt/b
On a slow connection, running 'df' and killing it while it's processing /mnt/b can make cifs_get_inode_info() returns -ERESTARTSYS.
This triggers the following chain of events: => the dentry revalidation fail => dentry is put and released => superblock associated with the dentry is put => /mnt/b is unmounted
This patch makes cifs_d_revalidate() return the error instead of 0 (invalid) when cifs_revalidate_dentry() fails, except for ENOENT (file deleted) and ESTALE (file recreated).
Signed-off-by: Aurelien Aptel aaptel@suse.com Suggested-by: Shyam Prasad N nspmangalore@gmail.com CC: stable@vger.kernel.org
fs/cifs/dir.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 68900f1629bff..97ac363b5df16 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -737,6 +737,7 @@ static int cifs_d_revalidate(struct dentry *direntry, unsigned int flags) { struct inode *inode;
int rc; if (flags & LOOKUP_RCU) return -ECHILD;@@ -746,8 +747,25 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags) if ((flags & LOOKUP_REVAL) && !CIFS_CACHE_READ(CIFS_I(inode))) CIFS_I(inode)->time = 0; /* force reval */
if (cifs_revalidate_dentry(direntry))return 0;
rc = cifs_revalidate_dentry(direntry);if (rc) {cifs_dbg(FYI, "cifs_revalidate_dentry failed with rc=%d", rc);switch (rc) {case -ENOENT:case -ESTALE:/** Those errors mean the dentry is invalid* (file was deleted or recreated)*/return 0;default:/** Otherwise some unexpected error happened* report it as-is to VFS layer*/return rc;}} else { /* * If the inode wasn't known to be a dfs entry when-- 2.29.2
On Fri, Feb 5, 2021 at 8:52 AM Shyam Prasad N nspmangalore@gmail.com wrote:
Looks good to me.
We do need to find out how a single client test could generate ESTALE though (perhaps we forgot to drop a dentry on delete or rmdir). The test is doing fsstress, so should be easy enough to shorten it and repro and find out exactly why the ESTALE is coming.
Maybe change the FYI in the cifs_dbg line above to VFS?
Probably safer to put a dynamic trace point there in case EACCES or other rc becomes too common in some scenario.
On Fri, Feb 5, 2021 at 6:42 AM Aurélien Aptel aaptel@suse.com wrote:
From: Aurelien Aptel aaptel@suse.com
Assuming
- //HOST/a is mounted on /mnt
- //HOST/b is mounted on /mnt/b
On a slow connection, running 'df' and killing it while it's processing /mnt/b can make cifs_get_inode_info() returns -ERESTARTSYS.
This triggers the following chain of events: => the dentry revalidation fail => dentry is put and released => superblock associated with the dentry is put => /mnt/b is unmounted
This patch makes cifs_d_revalidate() return the error instead of 0 (invalid) when cifs_revalidate_dentry() fails, except for ENOENT (file deleted) and ESTALE (file recreated).
Signed-off-by: Aurelien Aptel aaptel@suse.com Suggested-by: Shyam Prasad N nspmangalore@gmail.com CC: stable@vger.kernel.org
fs/cifs/dir.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 68900f1629bff..97ac363b5df16 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -737,6 +737,7 @@ static int cifs_d_revalidate(struct dentry *direntry, unsigned int flags) { struct inode *inode;
int rc; if (flags & LOOKUP_RCU) return -ECHILD;@@ -746,8 +747,25 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags) if ((flags & LOOKUP_REVAL) && !CIFS_CACHE_READ(CIFS_I(inode))) CIFS_I(inode)->time = 0; /* force reval */
if (cifs_revalidate_dentry(direntry))return 0;
rc = cifs_revalidate_dentry(direntry);if (rc) {cifs_dbg(FYI, "cifs_revalidate_dentry failed with rc=%d", rc);switch (rc) {case -ENOENT:case -ESTALE:/** Those errors mean the dentry is invalid* (file was deleted or recreated)*/return 0;default:/** Otherwise some unexpected error happened* report it as-is to VFS layer*/return rc;}} else { /* * If the inode wasn't known to be a dfs entry when-- 2.29.2
-- Regards, Shyam
Tentatively merged into cifs-2.6.git for-next, since it seems to have fixed the problem we saw with test generic/070
On Fri, Feb 5, 2021 at 8:42 AM Aurélien Aptel aaptel@suse.com wrote:
From: Aurelien Aptel aaptel@suse.com
Assuming
- //HOST/a is mounted on /mnt
- //HOST/b is mounted on /mnt/b
On a slow connection, running 'df' and killing it while it's processing /mnt/b can make cifs_get_inode_info() returns -ERESTARTSYS.
This triggers the following chain of events: => the dentry revalidation fail => dentry is put and released => superblock associated with the dentry is put => /mnt/b is unmounted
This patch makes cifs_d_revalidate() return the error instead of 0 (invalid) when cifs_revalidate_dentry() fails, except for ENOENT (file deleted) and ESTALE (file recreated).
Signed-off-by: Aurelien Aptel aaptel@suse.com Suggested-by: Shyam Prasad N nspmangalore@gmail.com CC: stable@vger.kernel.org
fs/cifs/dir.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 68900f1629bff..97ac363b5df16 100644 --- a/fs/cifs/dir.c +++ b/fs/cifs/dir.c @@ -737,6 +737,7 @@ static int cifs_d_revalidate(struct dentry *direntry, unsigned int flags) { struct inode *inode;
int rc; if (flags & LOOKUP_RCU) return -ECHILD;@@ -746,8 +747,25 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags) if ((flags & LOOKUP_REVAL) && !CIFS_CACHE_READ(CIFS_I(inode))) CIFS_I(inode)->time = 0; /* force reval */
if (cifs_revalidate_dentry(direntry))return 0;
rc = cifs_revalidate_dentry(direntry);if (rc) {cifs_dbg(FYI, "cifs_revalidate_dentry failed with rc=%d", rc);switch (rc) {case -ENOENT:case -ESTALE:/** Those errors mean the dentry is invalid* (file was deleted or recreated)*/return 0;default:/** Otherwise some unexpected error happened* report it as-is to VFS layer*/return rc;}} else { /* * If the inode wasn't known to be a dfs entry when-- 2.29.2
-- Thanks,
Steve
linux-stable-mirror@lists.linaro.org
-
Aurélien Aptel -
Shyam Prasad N -
Steve French