[PATCH] ext3: call blkdev_issue_flush on fsync

Previous thread: Re: [PATCH 3/3] ext4: symlink must be handled via filesystem specific operation by tytso on Sunday, April 4, 2010 - 1:05 pm. (10 messages)

Next thread: Re: [PATCH] ext4: use __ext4_get_inode_loc() from ext4_write_inode() by tytso on Saturday, April 3, 2010 - 2:28 pm. (2 messages)
From: Surbhi Palande
Date: Friday, March 26, 2010 - 9:50 am

To ensure that bits are truly on-disk after an fsync,
we should call blkdev_issue_flush if barriers are supported.

This code is seen in ext4 through commits
d755fb384250d6bd7fd18a0930e71965acc8e72e and
5f3481e9a80c240f169b36ea886e2325b9aeb745.

Signed-off-by: Surbhi Palande <surbhi.palande@canonical.com>
---
 fs/ext3/fsync.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fs/ext3/fsync.c b/fs/ext3/fsync.c
index d336341..2184a40 100644
--- a/fs/ext3/fsync.c
+++ b/fs/ext3/fsync.c
@@ -29,6 +29,7 @@
 #include <linux/jbd.h>
 #include <linux/ext3_fs.h>
 #include <linux/ext3_jbd.h>
+#include <linux/blkdev.h>
 
 /*
  * akpm: A new design for ext3_sync_file().
@@ -46,6 +47,7 @@ int ext3_sync_file(struct file * file, struct dentry *dentry, int datasync)
 {
 	struct inode *inode = dentry->d_inode;
 	int ret = 0;
+	journal_t *journal = EXT3_SB(inode->i_sb)->s_journal;
 
 	J_ASSERT(ext3_journal_current_handle() == NULL);
 
@@ -87,5 +89,7 @@ int ext3_sync_file(struct file * file, struct dentry *dentry, int datasync)
 		ret = sync_inode(inode, &wbc);
 	}
 out:
+	if (journal && (journal->j_flags & JFS_BARRIER))
+		blkdev_issue_flush(inode->i_sb->s_bdev, NULL);
 	return ret;
 }
-- 
1.6.3.3

--

From: Andreas Dilger
Date: Friday, March 26, 2010 - 12:24 pm

I don't think we need yet ANOTHER barrier here.  If the filesystem is  
mounted in data={journaled,ordered} mode it will have flushed the data  
to disk as part of the journal commit.  If there is an external  
journal, there were patches posted to have it flush the data on the  
filesystem device at transaction commit time.

Since fsync on any inode always implies sync of the journal, the only  
time that this would be needed is if we are running in no-journal  
mode, or possibly in data=writeback mode.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--

From: Eric Sandeen
Date: Friday, March 26, 2010 - 12:37 pm

And no-journal mode isn't possible in ext3 :)

Actually unless I'm totally confused, this patch doesn't apply at all,
and we already have:

        if (log_start_commit(journal, commit_tid)) {
                log_wait_commit(journal, commit_tid);
                goto out;
        }

        /*
         * In case we didn't commit a transaction, we have to flush
         * disk caches manually so that data really is on persistent
         * storage
         */
        if (test_opt(inode->i_sb, BARRIER))
                blkdev_issue_flush(inode->i_sb->s_bdev, NULL);
out:
        return ret;

in ext3_sync_file(), from commit 56fcad29d4b3cbcbb2ed47a9d3ceca3f57175417...

-Eric
--

From: Surbhi Palande
Date: Friday, March 26, 2010 - 12:55 pm

Please ignore this patch! Sorry for this!

Warm Regards,
Surbhi.



--

Previous thread: Re: [PATCH 3/3] ext4: symlink must be handled via filesystem specific operation by tytso on Sunday, April 4, 2010 - 1:05 pm. (10 messages)

Next thread: Re: [PATCH] ext4: use __ext4_get_inode_loc() from ext4_write_inode() by tytso on Saturday, April 3, 2010 - 2:28 pm. (2 messages)