[PATCH] ext3: call blkdev_issue_flush on fsync

[PATCH] ext3: call blkdev_issue_flush on fsync

[Surbhi Palande]

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

Re: [PATCH] ext3: call blkdev_issue_flush on fsync

[Andreas Dilger]

On 2010-03-26, at 10:50, Surbhi Palande wrote:
> To ensure that bits are truly on-disk after an fsync,
> we should call blkdev_issue_flush if barriers are supported.
>
> @@ -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;


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.

Re: [PATCH] ext3: call blkdev_issue_flush on fsync

[Eric Sandeen]

On 03/26/2010 02:24 PM, Andreas Dilger wrote:
> On 2010-03-26, at 10:50, Surbhi Palande wrote:
>> To ensure that bits are truly on-disk after an fsync,
>> we should call blkdev_issue_flush if barriers are supported.
>>
>> @@ -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;
> 
> 
> 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.

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

Re: [PATCH] ext3: call blkdev_issue_flush on fsync

[Surbhi Palande]

Please ignore this patch! Sorry for this!

Warm Regards,
Surbhi.

On Fri, 2010-03-26 at 18:50 +0200, Surbhi Palande wrote:
> 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;
>  }