make sure data is on disk before calling ->write_inode
authorChristoph Hellwig <hch@lst.de>
Fri, 5 Mar 2010 08:21:21 +0000 (09:21 +0100)
committerAl Viro <viro@zeniv.linux.org.uk>
Fri, 5 Mar 2010 18:25:10 +0000 (13:25 -0500)
Similar to the fsync issue fixed a while ago in commit
2daea67e966dc0c42067ebea015ddac6834cef88 we need to write for data to
actually hit the disk before writing out the metadata to guarantee
data integrity for filesystems that modify the inode in the data I/O
completion path.  Currently XFS and NFS handle this manually, and AFS
has a write_inode method that does nothing but waiting for data, while
others are possibly missing out on this.

Fortunately this change has a lot less impact than the fsync change
as none of the write_inode methods starts data writeout of any form
by itself.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/afs/internal.h
fs/afs/super.c
fs/afs/write.c
fs/fs-writeback.c
fs/nfs/inode.c
fs/xfs/linux-2.6/xfs_super.c

index 6ece2a13bf711fd477098d18b60a34a4861fea59..c54dad4e60636ac687608cacc2e5baf70bcb80b0 100644 (file)
@@ -733,7 +733,6 @@ extern int afs_write_end(struct file *file, struct address_space *mapping,
                        struct page *page, void *fsdata);
 extern int afs_writepage(struct page *, struct writeback_control *);
 extern int afs_writepages(struct address_space *, struct writeback_control *);
-extern int afs_write_inode(struct inode *, int);
 extern void afs_pages_written_back(struct afs_vnode *, struct afs_call *);
 extern ssize_t afs_file_write(struct kiocb *, const struct iovec *,
                              unsigned long, loff_t);
index e1ea1c240b6a2caa7e6b5220bc02380db0e2dea5..14f6431598adfc57d4b563ca98bb40116f87b1ed 100644 (file)
@@ -48,7 +48,6 @@ struct file_system_type afs_fs_type = {
 static const struct super_operations afs_super_ops = {
        .statfs         = afs_statfs,
        .alloc_inode    = afs_alloc_inode,
-       .write_inode    = afs_write_inode,
        .destroy_inode  = afs_destroy_inode,
        .clear_inode    = afs_clear_inode,
        .put_super      = afs_put_super,
index 5e15a21dbf9f3f9d705c2889761e04749c45531a..3bed54a294d4300e234e9c65b49338aced33c7f2 100644 (file)
@@ -584,27 +584,6 @@ int afs_writepages(struct address_space *mapping,
        return ret;
 }
 
-/*
- * write an inode back
- */
-int afs_write_inode(struct inode *inode, int sync)
-{
-       struct afs_vnode *vnode = AFS_FS_I(inode);
-       int ret;
-
-       _enter("{%x:%u},", vnode->fid.vid, vnode->fid.vnode);
-
-       ret = 0;
-       if (sync) {
-               ret = filemap_fdatawait(inode->i_mapping);
-               if (ret < 0)
-                       __mark_inode_dirty(inode, I_DIRTY_DATASYNC);
-       }
-
-       _leave(" = %d", ret);
-       return ret;
-}
-
 /*
  * completion of write to server
  */
index 1a7c42c64ff47ee7e55c378d0e067892d789e2d7..5f2721b1e4beeb46e6fee3841fbab5cab7dc463f 100644 (file)
@@ -461,15 +461,20 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
 
        ret = do_writepages(mapping, wbc);
 
-       /* Don't write the inode if only I_DIRTY_PAGES was set */
-       if (dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
-               int err = write_inode(inode, wait);
+       /*
+        * Make sure to wait on the data before writing out the metadata.
+        * This is important for filesystems that modify metadata on data
+        * I/O completion.
+        */
+       if (wait) {
+               int err = filemap_fdatawait(mapping);
                if (ret == 0)
                        ret = err;
        }
 
-       if (wait) {
-               int err = filemap_fdatawait(mapping);
+       /* Don't write the inode if only I_DIRTY_PAGES was set */
+       if (dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
+               int err = write_inode(inode, wait);
                if (ret == 0)
                        ret = err;
        }
index 7570573bdb30dc670538e53951a1cf1a8d1a8d06..5ecd952cae1d5529b459e4e5a5a873fd194abd97 100644 (file)
@@ -101,12 +101,7 @@ int nfs_write_inode(struct inode *inode, int sync)
 {
        int ret;
 
-       if (sync) {
-               ret = filemap_fdatawait(inode->i_mapping);
-               if (ret == 0)
-                       ret = nfs_commit_inode(inode, FLUSH_SYNC);
-       } else
-               ret = nfs_commit_inode(inode, 0);
+       ret = nfs_commit_inode(inode, sync ? FLUSH_SYNC : 0);
        if (ret >= 0)
                return 0;
        __mark_inode_dirty(inode, I_DIRTY_DATASYNC);
index 25ea2408118fb0fba760ef955922eed742116ee2..8f117db6070e627c9ed5ad8edc7830d5d5666df4 100644 (file)
@@ -1075,10 +1075,6 @@ xfs_fs_write_inode(
                return XFS_ERROR(EIO);
 
        if (sync) {
-               error = xfs_wait_on_pages(ip, 0, -1);
-               if (error)
-                       goto out;
-
                /*
                 * Make sure the inode has hit stable storage.  By using the
                 * log and the fsync transactions we reduce the IOs we have