>From fdatasync(2):
"fdatasync() is similar to fsync(), but does not flush modified metadata
unless that metadata is needed in order to allow a subsequent data
retrieval to be correctly handled."
We absolutely need to commit the layouts to be able to retrieve the data
in case either the client, the server or the storage subsystem go down.
Signed-off-by: Christoph Hellwig <[email protected]>
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 8de3407..464db9d 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -100,8 +100,7 @@ nfs4_file_fsync(struct file *file, loff_t start, loff_t end, int datasync)
break;
mutex_lock(&inode->i_mutex);
ret = nfs_file_fsync_commit(file, start, end, datasync);
- if (!ret && !datasync)
- /* application has asked for meta-data sync */
+ if (!ret)
ret = pnfs_layoutcommit_inode(inode, true);
mutex_unlock(&inode->i_mutex);
/*
On Sun, May 04, 2014 at 11:56:24PM -0700, Christoph Hellwig wrote:
> ping? This is a fairly serious data integrity issue for pnfs users.
ping^2
without this fdatasync and O_DSYNC writes on pnfs are more or less
noops, so at least getting a review would be helpful, nevermind
forwarding it to Linus and -stable.
>
> On Mon, Apr 21, 2014 at 10:29:17AM -0700, Christoph Hellwig wrote:
> > >From fdatasync(2):
> >
> > "fdatasync() is similar to fsync(), but does not flush modified metadata
> > unless that metadata is needed in order to allow a subsequent data
> > retrieval to be correctly handled."
> >
> > We absolutely need to commit the layouts to be able to retrieve the data
> > in case either the client, the server or the storage subsystem go down.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
> >
> > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> > index 8de3407..464db9d 100644
> > --- a/fs/nfs/nfs4file.c
> > +++ b/fs/nfs/nfs4file.c
> > @@ -100,8 +100,7 @@ nfs4_file_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> > break;
> > mutex_lock(&inode->i_mutex);
> > ret = nfs_file_fsync_commit(file, start, end, datasync);
> > - if (!ret && !datasync)
> > - /* application has asked for meta-data sync */
> > + if (!ret)
> > ret = pnfs_layoutcommit_inode(inode, true);
> > mutex_unlock(&inode->i_mutex);
> > /*
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> ---end quoted text---
---end quoted text---
On Tue, May 27, 2014 at 12:00 PM, Christoph Hellwig <[email protected]> wrote:
> On Sun, May 04, 2014 at 11:56:24PM -0700, Christoph Hellwig wrote:
>> ping? This is a fairly serious data integrity issue for pnfs users.
>
> ping^2
>
> without this fdatasync and O_DSYNC writes on pnfs are more or less
> noops, so at least getting a review would be helpful, nevermind
> forwarding it to Linus and -stable.
Applied for 3.16. Not sure about -stable eligibility, since this only
affects pnfs blocks for now (files and objects should both be able to
recover in case of client failure). Are you seeing this in production
environments?
Cheers
Trond
>> On Mon, Apr 21, 2014 at 10:29:17AM -0700, Christoph Hellwig wrote:
>> > >From fdatasync(2):
>> >
>> > "fdatasync() is similar to fsync(), but does not flush modified metadata
>> > unless that metadata is needed in order to allow a subsequent data
>> > retrieval to be correctly handled."
>> >
>> > We absolutely need to commit the layouts to be able to retrieve the data
>> > in case either the client, the server or the storage subsystem go down.
>> >
>> > Signed-off-by: Christoph Hellwig <[email protected]>
>> >
>> > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
>> > index 8de3407..464db9d 100644
>> > --- a/fs/nfs/nfs4file.c
>> > +++ b/fs/nfs/nfs4file.c
>> > @@ -100,8 +100,7 @@ nfs4_file_fsync(struct file *file, loff_t start, loff_t end, int datasync)
>> > break;
>> > mutex_lock(&inode->i_mutex);
>> > ret = nfs_file_fsync_commit(file, start, end, datasync);
>> > - if (!ret && !datasync)
>> > - /* application has asked for meta-data sync */
>> > + if (!ret)
>> > ret = pnfs_layoutcommit_inode(inode, true);
>> > mutex_unlock(&inode->i_mutex);
>> > /*
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> > the body of a message to [email protected]
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> ---end quoted text---
> ---end quoted text---
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]
ping? This is a fairly serious data integrity issue for pnfs users.
On Mon, Apr 21, 2014 at 10:29:17AM -0700, Christoph Hellwig wrote:
> >From fdatasync(2):
>
> "fdatasync() is similar to fsync(), but does not flush modified metadata
> unless that metadata is needed in order to allow a subsequent data
> retrieval to be correctly handled."
>
> We absolutely need to commit the layouts to be able to retrieve the data
> in case either the client, the server or the storage subsystem go down.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
>
> diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> index 8de3407..464db9d 100644
> --- a/fs/nfs/nfs4file.c
> +++ b/fs/nfs/nfs4file.c
> @@ -100,8 +100,7 @@ nfs4_file_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> break;
> mutex_lock(&inode->i_mutex);
> ret = nfs_file_fsync_commit(file, start, end, datasync);
> - if (!ret && !datasync)
> - /* application has asked for meta-data sync */
> + if (!ret)
> ret = pnfs_layoutcommit_inode(inode, true);
> mutex_unlock(&inode->i_mutex);
> /*
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
---end quoted text---
On Wed, May 28, 2014 at 07:00:08PM -0400, Trond Myklebust wrote:
> Applied for 3.16. Not sure about -stable eligibility, since this only
> affects pnfs blocks for now (files and objects should both be able to
> recover in case of client failure).
At least the linux-nfs.org object server needs the layoutcommit to
update i_size on the inode, so changes won't be persistent without the
layoutcommit. I expect loosely coupled file servers to work in a
similar fashion.
> Are you seeing this in production
> environments?
This was found during data integrity testing.