linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] NFS: Don't refresh attributes with mounted-on-file information
@ 2019-08-13 14:28 Trond Myklebust
  2019-08-13 14:28 ` [PATCH 2/5] NFSv4: Fix return values for nfs4_file_open() Trond Myklebust
  0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2019-08-13 14:28 UTC (permalink / raw)
  To: linux-nfs

If we've been given the attributes of the mounted-on-file, then do not
use those to check or update the attributes on the application-visible
inode.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/inode.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 8a1758200b57..c764cfe456e5 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1403,12 +1403,21 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
 	if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ))
 		return 0;
 
+	/* No fileid? Just exit */
+	if (!(fattr->valid & NFS_ATTR_FATTR_FILEID))
+		return 0;
 	/* Has the inode gone and changed behind our back? */
-	if ((fattr->valid & NFS_ATTR_FATTR_FILEID) && nfsi->fileid != fattr->fileid)
+	if (nfsi->fileid != fattr->fileid) {
+		/* Is this perhaps the mounted-on fileid? */
+		if ((fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID) &&
+		    nfsi->fileid == fattr->mounted_on_fileid)
+			return 0;
 		return -ESTALE;
+	}
 	if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT))
 		return -ESTALE;
 
+
 	if (!nfs_file_has_buffered_writers(nfsi)) {
 		/* Verify a few of the more important attributes */
 		if ((fattr->valid & NFS_ATTR_FATTR_CHANGE) != 0 && !inode_eq_iversion_raw(inode, fattr->change_attr))
@@ -1768,18 +1777,6 @@ int nfs_post_op_update_inode_force_wcc(struct inode *inode, struct nfs_fattr *fa
 EXPORT_SYMBOL_GPL(nfs_post_op_update_inode_force_wcc);
 
 
-static inline bool nfs_fileid_valid(struct nfs_inode *nfsi,
-				    struct nfs_fattr *fattr)
-{
-	bool ret1 = true, ret2 = true;
-
-	if (fattr->valid & NFS_ATTR_FATTR_FILEID)
-		ret1 = (nfsi->fileid == fattr->fileid);
-	if (fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID)
-		ret2 = (nfsi->fileid == fattr->mounted_on_fileid);
-	return ret1 || ret2;
-}
-
 /*
  * Many nfs protocol calls return the new file attributes after
  * an operation.  Here we update the inode to reflect the state
@@ -1810,7 +1807,15 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 			nfs_display_fhandle_hash(NFS_FH(inode)),
 			atomic_read(&inode->i_count), fattr->valid);
 
-	if (!nfs_fileid_valid(nfsi, fattr)) {
+	/* No fileid? Just exit */
+	if (!(fattr->valid & NFS_ATTR_FATTR_FILEID))
+		return 0;
+	/* Has the inode gone and changed behind our back? */
+	if (nfsi->fileid != fattr->fileid) {
+		/* Is this perhaps the mounted-on fileid? */
+		if ((fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID) &&
+		    nfsi->fileid == fattr->mounted_on_fileid)
+			return 0;
 		printk(KERN_ERR "NFS: server %s error: fileid changed\n"
 			"fsid %s: expected fileid 0x%Lx, got 0x%Lx\n",
 			NFS_SERVER(inode)->nfs_client->cl_hostname,
-- 
2.21.0


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

* [PATCH 2/5] NFSv4: Fix return values for nfs4_file_open()
  2019-08-13 14:28 [PATCH 1/5] NFS: Don't refresh attributes with mounted-on-file information Trond Myklebust
@ 2019-08-13 14:28 ` Trond Myklebust
  2019-08-13 14:28   ` [PATCH 3/5] NFSv4: Fix return value in nfs_finish_open() Trond Myklebust
  0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2019-08-13 14:28 UTC (permalink / raw)
  To: linux-nfs

Currently, we are translating RPC level errors such as timeouts,
as well as interrupts etc into EOPENSTALE, which forces a single
replay of the open attempt. What we actually want to do is
force the replay only in the cases where the returned error
indicates that the file may have changed on the server.

So the fix is to spell out the exact set of errors where we want
to return EOPENSTALE.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/nfs4file.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 96db471ca2e5..339663d04bf8 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -73,13 +73,13 @@ nfs4_file_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(inode)) {
 		err = PTR_ERR(inode);
 		switch (err) {
-		case -EPERM:
-		case -EACCES:
-		case -EDQUOT:
-		case -ENOSPC:
-		case -EROFS:
-			goto out_put_ctx;
 		default:
+			goto out_put_ctx;
+		case -ENOENT:
+		case -ESTALE:
+		case -EISDIR:
+		case -ENOTDIR:
+		case -ELOOP:
 			goto out_drop;
 		}
 	}
-- 
2.21.0


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

* [PATCH 3/5] NFSv4: Fix return value in nfs_finish_open()
  2019-08-13 14:28 ` [PATCH 2/5] NFSv4: Fix return values for nfs4_file_open() Trond Myklebust
@ 2019-08-13 14:28   ` Trond Myklebust
  2019-08-13 14:28     ` [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend() Trond Myklebust
  0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2019-08-13 14:28 UTC (permalink / raw)
  To: linux-nfs

If the file turns out to be of the wrong type after opening, we want
to revalidate the path and retry, so return EOPENSTALE rather than
ESTALE.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 8d501093660f..0adfd8840110 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1487,7 +1487,7 @@ static int nfs_finish_open(struct nfs_open_context *ctx,
 	if (S_ISREG(file->f_path.dentry->d_inode->i_mode))
 		nfs_file_set_open_context(file, ctx);
 	else
-		err = -ESTALE;
+		err = -EOPENSTALE;
 out:
 	return err;
 }
-- 
2.21.0


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

* [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend()
  2019-08-13 14:28   ` [PATCH 3/5] NFSv4: Fix return value in nfs_finish_open() Trond Myklebust
@ 2019-08-13 14:28     ` Trond Myklebust
  2019-08-13 14:28       ` [PATCH 5/5] NFS: Ensure O_DIRECT reports an error if the bytes read/written is 0 Trond Myklebust
  2019-08-13 17:55       ` [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend() Sasha Levin
  0 siblings, 2 replies; 6+ messages in thread
From: Trond Myklebust @ 2019-08-13 14:28 UTC (permalink / raw)
  To: linux-nfs

If the attempt to resend the pages fails, we need to ensure that we
clean up those pages that were not transmitted.

Fixes: d600ad1f2bdb ("NFS41: pop some layoutget errors to application")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: stable@vger.kernel.org # v4.5+
---
 fs/nfs/pagelist.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index ed4e1b07447b..15c254753f88 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -1251,20 +1251,22 @@ static void nfs_pageio_complete_mirror(struct nfs_pageio_descriptor *desc,
 int nfs_pageio_resend(struct nfs_pageio_descriptor *desc,
 		      struct nfs_pgio_header *hdr)
 {
-	LIST_HEAD(failed);
+	LIST_HEAD(pages);
 
 	desc->pg_io_completion = hdr->io_completion;
 	desc->pg_dreq = hdr->dreq;
-	while (!list_empty(&hdr->pages)) {
-		struct nfs_page *req = nfs_list_entry(hdr->pages.next);
+	list_splice_init(&hdr->pages, &pages);
+	while (!list_empty(&pages)) {
+		struct nfs_page *req = nfs_list_entry(pages.next);
 
 		if (!nfs_pageio_add_request(desc, req))
-			nfs_list_move_request(req, &failed);
+			break;
 	}
 	nfs_pageio_complete(desc);
-	if (!list_empty(&failed)) {
-		list_move(&failed, &hdr->pages);
-		return desc->pg_error < 0 ? desc->pg_error : -EIO;
+	if (!list_empty(&pages)) {
+		int err = desc->pg_error < 0 ? desc->pg_error : -EIO;
+		hdr->completion_ops->error_cleanup(&pages, err);
+		return err;
 	}
 	return 0;
 }
-- 
2.21.0


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

* [PATCH 5/5] NFS: Ensure O_DIRECT reports an error if the bytes read/written is 0
  2019-08-13 14:28     ` [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend() Trond Myklebust
@ 2019-08-13 14:28       ` Trond Myklebust
  2019-08-13 17:55       ` [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend() Sasha Levin
  1 sibling, 0 replies; 6+ messages in thread
From: Trond Myklebust @ 2019-08-13 14:28 UTC (permalink / raw)
  To: linux-nfs

If the attempt to resend the I/O results in no bytes being read/written,
we must ensure that we report the error.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Fixes: 0a00b77b331a ("nfs: mirroring support for direct io")
Cc: stable@vger.kernel.org # v3.20+
---
 fs/nfs/direct.c   | 27 ++++++++++++++++++---------
 fs/nfs/pagelist.c |  1 +
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 0cb442406168..222d7115db71 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -401,15 +401,21 @@ static void nfs_direct_read_completion(struct nfs_pgio_header *hdr)
 	unsigned long bytes = 0;
 	struct nfs_direct_req *dreq = hdr->dreq;
 
-	if (test_bit(NFS_IOHDR_REDO, &hdr->flags))
-		goto out_put;
-
 	spin_lock(&dreq->lock);
-	if (test_bit(NFS_IOHDR_ERROR, &hdr->flags) && (hdr->good_bytes == 0))
+	if (test_bit(NFS_IOHDR_ERROR, &hdr->flags))
 		dreq->error = hdr->error;
-	else
+
+	if (test_bit(NFS_IOHDR_REDO, &hdr->flags)) {
+		spin_unlock(&dreq->lock);
+		goto out_put;
+	}
+
+	if (hdr->good_bytes != 0)
 		nfs_direct_good_bytes(dreq, hdr);
 
+	if (test_bit(NFS_IOHDR_EOF, &hdr->flags))
+		dreq->error = 0;
+
 	spin_unlock(&dreq->lock);
 
 	while (!list_empty(&hdr->pages)) {
@@ -782,16 +788,19 @@ static void nfs_direct_write_completion(struct nfs_pgio_header *hdr)
 	bool request_commit = false;
 	struct nfs_page *req = nfs_list_entry(hdr->pages.next);
 
-	if (test_bit(NFS_IOHDR_REDO, &hdr->flags))
-		goto out_put;
-
 	nfs_init_cinfo_from_dreq(&cinfo, dreq);
 
 	spin_lock(&dreq->lock);
 
 	if (test_bit(NFS_IOHDR_ERROR, &hdr->flags))
 		dreq->error = hdr->error;
-	if (dreq->error == 0) {
+
+	if (test_bit(NFS_IOHDR_REDO, &hdr->flags)) {
+		spin_unlock(&dreq->lock);
+		goto out_put;
+	}
+
+	if (hdr->good_bytes != 0) {
 		nfs_direct_good_bytes(dreq, hdr);
 		if (nfs_write_need_commit(hdr)) {
 			if (dreq->flags == NFS_ODIRECT_RESCHED_WRITES)
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 15c254753f88..56cefa0ab804 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -1266,6 +1266,7 @@ int nfs_pageio_resend(struct nfs_pageio_descriptor *desc,
 	if (!list_empty(&pages)) {
 		int err = desc->pg_error < 0 ? desc->pg_error : -EIO;
 		hdr->completion_ops->error_cleanup(&pages, err);
+		nfs_set_pgio_error(hdr, err, hdr->io_start);
 		return err;
 	}
 	return 0;
-- 
2.21.0


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

* Re: [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend()
  2019-08-13 14:28     ` [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend() Trond Myklebust
  2019-08-13 14:28       ` [PATCH 5/5] NFS: Ensure O_DIRECT reports an error if the bytes read/written is 0 Trond Myklebust
@ 2019-08-13 17:55       ` Sasha Levin
  1 sibling, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2019-08-13 17:55 UTC (permalink / raw)
  To: Sasha Levin, Trond Myklebust, linux-nfs; +Cc: stable

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: d600ad1f2bdb NFS41: pop some layoutget errors to application.

The bot has tested the following trees: v5.2.8, v4.19.66, v4.14.138, v4.9.189, v4.4.189.

v5.2.8: Build OK!
v4.19.66: Failed to apply! Possible dependencies:
    078b5fd92c49 ("NFS: Clean up list moves of struct nfs_page")

v4.14.138: Failed to apply! Possible dependencies:
    078b5fd92c49 ("NFS: Clean up list moves of struct nfs_page")

v4.9.189: Failed to apply! Possible dependencies:
    078b5fd92c49 ("NFS: Clean up list moves of struct nfs_page")

v4.4.189: Failed to apply! Possible dependencies:
    078b5fd92c49 ("NFS: Clean up list moves of struct nfs_page")
    6272dcc6beeb ("NFS: Simplify nfs_request_add_commit_list() arguments")
    b20135d0b243 ("NFSv4.1/pNFS: Don't queue up a new commit if the layout segment is invalid")
    c18b96a1b862 ("nfs: clean up rest of reqs when failing to add one")
    f57dcf4c7211 ("NFS: Fix I/O request leakages")
    fe238e601d25 ("NFS: Save struct inode COPYING CREDITS Documentation Kbuild Kconfig MAINTAINERS Makefile README REPORTING-BUGS arch block certs crypto drivers firmware fs include init ipc kernel lib mm net samples scripts security sound tools usr virt inside nfs_commit_info to clarify usage of i_lock")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks,
Sasha

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

end of thread, other threads:[~2019-08-13 17:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-13 14:28 [PATCH 1/5] NFS: Don't refresh attributes with mounted-on-file information Trond Myklebust
2019-08-13 14:28 ` [PATCH 2/5] NFSv4: Fix return values for nfs4_file_open() Trond Myklebust
2019-08-13 14:28   ` [PATCH 3/5] NFSv4: Fix return value in nfs_finish_open() Trond Myklebust
2019-08-13 14:28     ` [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend() Trond Myklebust
2019-08-13 14:28       ` [PATCH 5/5] NFS: Ensure O_DIRECT reports an error if the bytes read/written is 0 Trond Myklebust
2019-08-13 17:55       ` [PATCH 4/5] NFSv4/pnfs: Fix a page lock leak in nfs_pageio_resend() Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).