Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/3] CIFS: Gracefully handle QueryInfo errors during open
@ 2019-09-30 17:06 Pavel Shilovsky
  2019-09-30 17:06 ` [PATCH 2/3] CIFS: Force revalidate inode when dentry is stale Pavel Shilovsky
  2019-09-30 17:06 ` [PATCH 3/3] CIFS: Force reval dentry if LOOKUP_REVAL flag is set Pavel Shilovsky
  0 siblings, 2 replies; 4+ messages in thread
From: Pavel Shilovsky @ 2019-09-30 17:06 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French, Pavel Shilovskiy

Currently if the client identifies problems when processing
metadata returned in CREATE response, the open handle is being
leaked. This causes multiple problems like a file missing a lease
break by that client which causes high latencies to other clients
accessing the file. Another side-effect of this is that the file
can't be deleted.

Fix this by closing the file after the client hits an error after
the file was opened and the open descriptor wasn't returned to
the user space. Also convert -ESTALE to -EOPENSTALE to allow
the VFS to revalidate a dentry and retry the open.

Cc: <stable@vger.kernel.org>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 97090693d182..168b76de193a 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -253,6 +253,12 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
 		rc = cifs_get_inode_info(&inode, full_path, buf, inode->i_sb,
 					 xid, fid);
 
+	if (rc) {
+		server->ops->close(xid, tcon, fid);
+		if (rc == -ESTALE)
+			rc = -EOPENSTALE;
+	}
+
 out:
 	kfree(buf);
 	return rc;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/3] CIFS: Force revalidate inode when dentry is stale
  2019-09-30 17:06 [PATCH 1/3] CIFS: Gracefully handle QueryInfo errors during open Pavel Shilovsky
@ 2019-09-30 17:06 ` Pavel Shilovsky
  2019-09-30 17:06 ` [PATCH 3/3] CIFS: Force reval dentry if LOOKUP_REVAL flag is set Pavel Shilovsky
  1 sibling, 0 replies; 4+ messages in thread
From: Pavel Shilovsky @ 2019-09-30 17:06 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French, Pavel Shilovskiy

Currently the client indicates that a dentry is stale when inode
numbers or type types between a local inode and a remote file
don't match. If this is the case attributes is not being copied
from remote to local, so, it is already known that the local copy
has stale metadata. That's why the inode needs to be marked for
revalidation in order to tell the VFS to lookup the dentry again
before openning a file. This prevents unexpected stale errors
to be returned to the user space when openning a file.

Cc: <stable@vger.kernel.org>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/inode.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 56ca4b8ccaba..79d9a60f21ba 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -414,6 +414,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
 		/* if uniqueid is different, return error */
 		if (unlikely(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM &&
 		    CIFS_I(*pinode)->uniqueid != fattr.cf_uniqueid)) {
+			CIFS_I(*pinode)->time = 0; /* force reval */
 			rc = -ESTALE;
 			goto cgiiu_exit;
 		}
@@ -421,6 +422,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
 		/* if filetype is different, return error */
 		if (unlikely(((*pinode)->i_mode & S_IFMT) !=
 		    (fattr.cf_mode & S_IFMT))) {
+			CIFS_I(*pinode)->time = 0; /* force reval */
 			rc = -ESTALE;
 			goto cgiiu_exit;
 		}
@@ -924,6 +926,7 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
 		/* if uniqueid is different, return error */
 		if (unlikely(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM &&
 		    CIFS_I(*inode)->uniqueid != fattr.cf_uniqueid)) {
+			CIFS_I(*inode)->time = 0; /* force reval */
 			rc = -ESTALE;
 			goto cgii_exit;
 		}
@@ -931,6 +934,7 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
 		/* if filetype is different, return error */
 		if (unlikely(((*inode)->i_mode & S_IFMT) !=
 		    (fattr.cf_mode & S_IFMT))) {
+			CIFS_I(*inode)->time = 0; /* force reval */
 			rc = -ESTALE;
 			goto cgii_exit;
 		}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3/3] CIFS: Force reval dentry if LOOKUP_REVAL flag is set
  2019-09-30 17:06 [PATCH 1/3] CIFS: Gracefully handle QueryInfo errors during open Pavel Shilovsky
  2019-09-30 17:06 ` [PATCH 2/3] CIFS: Force revalidate inode when dentry is stale Pavel Shilovsky
@ 2019-09-30 17:06 ` Pavel Shilovsky
  2019-10-10 18:06   ` Pavel Shilovsky
  1 sibling, 1 reply; 4+ messages in thread
From: Pavel Shilovsky @ 2019-09-30 17:06 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French, Pavel Shilovskiy

Mark inode for force revalidation if LOOKUP_REVAL flag is set.
This tells the client to actually send a QueryInfo request to
the server to obtain the latest metadata in case a directory
or a file were changed remotely.

Cc: <stable@vger.kernel.org>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/dir.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index be424e81e3ad..91a46b01d748 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -738,10 +738,16 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 static int
 cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
 {
+	struct inode *inode;
+
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
 	if (d_really_is_positive(direntry)) {
+		inode = d_inode(direntry);
+		if (flags & LOOKUP_REVAL)
+			CIFS_I(inode)->time = 0; /* force reval */
+
 		if (cifs_revalidate_dentry(direntry))
 			return 0;
 		else {
@@ -752,7 +758,7 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
 			 * attributes will have been updated by
 			 * cifs_revalidate_dentry().
 			 */
-			if (IS_AUTOMOUNT(d_inode(direntry)) &&
+			if (IS_AUTOMOUNT(inode) &&
 			   !(direntry->d_flags & DCACHE_NEED_AUTOMOUNT)) {
 				spin_lock(&direntry->d_lock);
 				direntry->d_flags |= DCACHE_NEED_AUTOMOUNT;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] CIFS: Force reval dentry if LOOKUP_REVAL flag is set
  2019-09-30 17:06 ` [PATCH 3/3] CIFS: Force reval dentry if LOOKUP_REVAL flag is set Pavel Shilovsky
@ 2019-10-10 18:06   ` Pavel Shilovsky
  0 siblings, 0 replies; 4+ messages in thread
From: Pavel Shilovsky @ 2019-10-10 18:06 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French, Pavel Shilovskiy

The updated version of this has been merged into for-next:

https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commitdiff;h=0b3d0ef9840f7be202393ca9116b857f6f793715

--
Best regards,
Pavel Shilovsky

пн, 30 сент. 2019 г. в 10:06, Pavel Shilovsky <piastryyy@gmail.com>:
>
> Mark inode for force revalidation if LOOKUP_REVAL flag is set.
> This tells the client to actually send a QueryInfo request to
> the server to obtain the latest metadata in case a directory
> or a file were changed remotely.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
> ---
>  fs/cifs/dir.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index be424e81e3ad..91a46b01d748 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -738,10 +738,16 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
>  static int
>  cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
>  {
> +       struct inode *inode;
> +
>         if (flags & LOOKUP_RCU)
>                 return -ECHILD;
>
>         if (d_really_is_positive(direntry)) {
> +               inode = d_inode(direntry);
> +               if (flags & LOOKUP_REVAL)
> +                       CIFS_I(inode)->time = 0; /* force reval */
> +
>                 if (cifs_revalidate_dentry(direntry))
>                         return 0;
>                 else {
> @@ -752,7 +758,7 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
>                          * attributes will have been updated by
>                          * cifs_revalidate_dentry().
>                          */
> -                       if (IS_AUTOMOUNT(d_inode(direntry)) &&
> +                       if (IS_AUTOMOUNT(inode) &&
>                            !(direntry->d_flags & DCACHE_NEED_AUTOMOUNT)) {
>                                 spin_lock(&direntry->d_lock);
>                                 direntry->d_flags |= DCACHE_NEED_AUTOMOUNT;
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30 17:06 [PATCH 1/3] CIFS: Gracefully handle QueryInfo errors during open Pavel Shilovsky
2019-09-30 17:06 ` [PATCH 2/3] CIFS: Force revalidate inode when dentry is stale Pavel Shilovsky
2019-09-30 17:06 ` [PATCH 3/3] CIFS: Force reval dentry if LOOKUP_REVAL flag is set Pavel Shilovsky
2019-10-10 18:06   ` Pavel Shilovsky

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org linux-cifs@archiver.kernel.org
	public-inbox-index linux-cifs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox