| Message ID | 20210202111607.16372-1-aaptel@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] cifs: report error instead of invalid when revalidating a dentry fails | expand |
What does cifs_revalidate_dentry return when the dentry is no longer exists? I'm guessing that it'll return some error (ENOENT?). Do we need to treat that as a special case and return 0 (invalid)? Regards, Shyam On Tue, Feb 2, 2021 at 4:46 PM 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. > > Signed-off-by: Aurelien Aptel <aaptel@suse.com> > Suggested-by: Shyam Prasad N <nspmangalore@gmail.com> > --- > fs/cifs/dir.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > index 68900f1629bff..4174f35590e62 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,11 @@ 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); > + return rc; > + } > else { > /* > * If the inode wasn't known to be a dfs entry when > -- > 2.29.2 >
Shyam Prasad N <nspmangalore@gmail.com> writes: > What does cifs_revalidate_dentry return when the dentry is no longer exists? > I'm guessing that it'll return some error (ENOENT?). Do we need to > treat that as a special case and return 0 (invalid)? Yes it seems to return ENOENT; I'll send a v3. Are there other errors we should consider? Cheers,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c index 68900f1629bff..4174f35590e62 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,11 @@ 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); + return rc; + } else { /* * If the inode wasn't known to be a dfs entry when