Re: [PATCH] ext3: call blkdev_issue_flush on fsync

[Eric Sandeen <sandeen@redhat.com>]

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