All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trond.myklebust@primarydata.com>
To: Scott Mayhew <smayhew@redhat.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: [PATCH 3/3] nfs: Fix cache_validity check in nfs_write_pageuptodate()
Date: Fri, 20 Jun 2014 13:41:23 -0400	[thread overview]
Message-ID: <1403286083-14345-4-git-send-email-trond.myklebust@primarydata.com> (raw)
In-Reply-To: <1403286083-14345-3-git-send-email-trond.myklebust@primarydata.com>

From: Scott Mayhew <smayhew@redhat.com>

NFS_INO_INVALID_DATA cannot be ignored, even if we have a delegation.

We're still having some problems with data corruption when multiple
clients are appending to a file and those clients are being granted
write delegations on open.

To reproduce:

Client A:
vi /mnt/`hostname -s`
while :; do echo "XXXXXXXXXXXXXXX" >>/mnt/file; sleep $(( $RANDOM % 5 )); done

Client B:
vi /mnt/`hostname -s`
while :; do echo "YYYYYYYYYYYYYYY" >>/mnt/file; sleep $(( $RANDOM % 5 )); done

What's happening is that in nfs_update_inode() we're recognizing that
the file size has changed and we're setting NFS_INO_INVALID_DATA
accordingly, but then we ignore the cache_validity flags in
nfs_write_pageuptodate() because we have a delegation.  As a result,
in nfs_updatepage() we're extending the write to cover the full page
even though we've not read in the data to begin with.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/write.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 48cca1a65a4b..8534ee5c207d 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -928,12 +928,14 @@ static bool nfs_write_pageuptodate(struct page *page, struct inode *inode)
 
 	if (nfs_have_delegated_attributes(inode))
 		goto out;
-	if (nfsi->cache_validity & (NFS_INO_INVALID_DATA|NFS_INO_REVAL_PAGECACHE))
+	if (nfsi->cache_validity & NFS_INO_REVAL_PAGECACHE)
 		return false;
 	smp_rmb();
 	if (test_bit(NFS_INO_INVALIDATING, &nfsi->flags))
 		return false;
 out:
+	if (nfsi->cache_validity & NFS_INO_INVALID_DATA)
+		return false;
 	return PageUptodate(page) != 0;
 }
 
-- 
1.9.3


  reply	other threads:[~2014-06-20 17:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-13 18:18 [PATCH RFC] nfs: ensure cached data is correct before using delegation Scott Mayhew
2014-06-13 18:18 ` Scott Mayhew
2014-06-13 21:19   ` Trond Myklebust
2014-06-17 21:04     ` Scott Mayhew
2014-06-17 21:37       ` Trond Myklebust
2014-06-20 12:51         ` Scott Mayhew
2014-06-20 17:41           ` [PATCH 0/3] " Trond Myklebust
2014-06-20 17:41             ` [PATCH 1/3] NFS: Clear NFS_INO_REVAL_PAGECACHE when we update the file size Trond Myklebust
2014-06-20 17:41               ` [PATCH 2/3] NFS: Don't mark the data cache as invalid if it has been flushed Trond Myklebust
2014-06-20 17:41                 ` Trond Myklebust [this message]
2014-06-23 13:55             ` [PATCH 0/3] nfs: ensure cached data is correct before using delegation Scott Mayhew

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1403286083-14345-4-git-send-email-trond.myklebust@primarydata.com \
    --to=trond.myklebust@primarydata.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=smayhew@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.