All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trond.myklebust@primarydata.com>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: Chris Perl <cperl@janestreet.com>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
	Chris Perl <chris.perl@gmail.com>
Subject: Re: File Read Returns Non-existent Null Bytes
Date: Wed, 25 Feb 2015 19:43:10 -0500	[thread overview]
Message-ID: <1424911390.41161.4.camel@primarydata.com> (raw)
In-Reply-To: <1424911067.41161.2.camel@primarydata.com>

On Wed, 2015-02-25 at 19:37 -0500, Trond Myklebust wrote:
> On Wed, 2015-02-25 at 17:32 -0500, Chuck Lever wrote:
> > FWIW it’s easy to reproduce a similar race with fsx, and I encounter
> > it frequently while running xfstests on fast NFS servers.
> > 
> > fsx invokes ftruncate following a set of asynchronous reads
> > (generated possibly due to readahead). The reads are started first,
> > then the SETATTR, but they complete out of order.
> > 
> > The SETATTR changes the test file’s size, and the completion
> > updates the file size in the client’s inode. Then the read requests
> > complete on the client and set the file’s size back to its old value.
> > 
> > All it takes is one late read completion, and the cached file size
> > is corrupted. fsx detects the file size mismatch and terminates the
> > test. The file size is corrected by a subsequent GETATTR (say, an
> > “ls -l” to check it after fsx has terminated).
> > 
> > While SETATTR blocks concurrent writes, there’s no serialization
> > on either the client or server to help guarantee the ordering of
> > SETATTR with read operations.
> > 
> > I’ve found a successful workaround by forcing the client to ignore
> > post-op attrs in read replies. A stronger solution might simply set
> > the “file attributes need update” flag in the inode if any file
> > attribute mutation is noticed during a read completion.
> 
> That's different. We definitely should aim to fix this kind of issue
> since you are talking about a single client being the only thing
> accessing the file on the server.
> How about the following patch?

Let me retry without the typos. The following will actually
compile... :-/

8<-------------------------------------------------------------
>From d8e01c1a6cf7cf29b5b03c98edf1a74007ae3119 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@primarydata.com>
Date: Wed, 25 Feb 2015 19:26:28 -0500
Subject: [PATCH] NFS: Quiesce reads before updating the file size after
 truncating

Chuck Lever reports seeing readaheads racing with truncate operations
and causing the file size to be reverted. Fix is to quiesce reads
after truncating the file on the server, but before updating the
size on the client.

Reported-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/inode.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 83107be3dd01..d0d74a72eb7d 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -565,6 +565,10 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset)
 	if (err)
 		goto out;
 
+	/* Quiesce reads before changing the file size */
+	invalidate_inode_pages2_range(inode->i_mapping,
+			offset >> PAGE_CACHE_SHIFT, -1);
+
 	spin_lock(&inode->i_lock);
 	i_size_write(inode, offset);
 	/* Optimisation */
-- 
2.1.0




  reply	other threads:[~2015-02-26  0:43 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-23 20:56 File Read Returns Non-existent Null Bytes Chris Perl
2015-02-23 22:34 ` Trond Myklebust
2015-02-25 17:04   ` Chris Perl
2015-02-25 17:37     ` Trond Myklebust
2015-02-25 21:02       ` Chris Perl
2015-02-25 21:47         ` Trond Myklebust
2015-02-25 21:53           ` Chris Perl
2015-02-25 22:15             ` Trond Myklebust
2015-02-26 12:41               ` Chris Perl
2015-02-26 13:29                 ` Trond Myklebust
2015-02-26 13:42                   ` Chris Perl
2015-02-26 14:10                     ` Chris Perl
2015-02-26 15:22                       ` Simo Sorce
2015-02-26 15:34                         ` Trond Myklebust
2015-02-26 15:36                           ` Simo Sorce
2015-02-26 15:45                             ` Chris Perl
2015-02-26 15:56                               ` Simo Sorce
2015-02-27  1:48                               ` Harshula
2015-02-27 13:17                                 ` Chris Perl
2015-02-26 16:00                           ` Chris Perl
2015-02-26 23:43                             ` Trond Myklebust
2015-02-26 15:37                         ` Trond Myklebust
2015-02-27 22:40                   ` J. Bruce Fields
2015-02-27 23:33                     ` Chuck Lever
2015-03-02 15:19                       ` Chris Perl
2015-03-02 15:57                         ` Chuck Lever
2015-03-02 20:58                           ` J. Bruce Fields
2015-03-02 21:15                             ` Chuck Lever
2015-03-03 13:29                               ` Chris Perl
2015-03-03 15:30                                 ` Chuck Lever
2015-03-03 17:44                                   ` Trond Myklebust
2015-03-03 19:57                                     ` Chuck Lever
2015-03-02 21:33                             ` didier
2015-03-03  9:09                               ` Boaz Harrosh
     [not found]                 ` <CAHHaOubVomDJ5uePb7DFGizZ0TBsyC-tJN5p6-RWOYKQC2oxvA@mail.gmail.com>
2015-02-27 20:13                   ` Chris Perl
2015-02-25 22:32           ` Chuck Lever
2015-02-26  0:37             ` Trond Myklebust
2015-02-26  0:43               ` Trond Myklebust [this message]
2015-02-26  1:27                 ` Trond Myklebust
2015-02-26 15:08                   ` Chuck Lever
2015-02-26 16:26                   ` fsx size error (was: File Read Returns Non-existent Null Bytes) Chuck Lever
2015-02-26 17:27                     ` Trond Myklebust
2015-02-26 19:00                       ` Chuck Lever
2015-02-26 23:06                         ` Trond Myklebust

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=1424911390.41161.4.camel@primarydata.com \
    --to=trond.myklebust@primarydata.com \
    --cc=chris.perl@gmail.com \
    --cc=chuck.lever@oracle.com \
    --cc=cperl@janestreet.com \
    --cc=linux-nfs@vger.kernel.org \
    /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.