| Message ID | 20140421172917.GA20036@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <hch@lst.de> > > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ---end quoted text--- -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <hch@lst.de> > > > > 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 majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > ---end quoted text--- ---end quoted text--- -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 27, 2014 at 12:00 PM, Christoph Hellwig <hch@infradead.org> 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 <hch@lst.de> >> > >> > 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 majordomo@vger.kernel.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> ---end quoted text--- > ---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. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); /*
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 <hch@lst.de> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html