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 <[email protected]>
---
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
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.
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
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 <[email protected]>
> ---
> 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;
> }