On 06/29/2011 12:38 PM, Jan Kara wrote:
> On Tue 28-06-11 11:35:10, Josef Bacik wrote:
>> Btrfs needs to be able to control how filemap_write_and_wait_range() is called
>> in fsync to make it less of a painful operation, so push down taking i_mutex and
>> the calling of filemap_write_and_wait() down into the ->fsync() handlers. Some
>> file systems can drop taking the i_mutex altogether it seems, like ext3 and
>> ocfs2. For correctness sake I just pushed everything down in all cases to make
>> sure that we keep the current behavior the same for everybody, and then each
>> individual fs maintainer can make up their mind about what to do from there.
>> Thanks,
>>
>> Signed-off-by: Josef Bacik <josef@xxxxxxxxxx>
> ...
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 1921392..fa44df8 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -1758,7 +1758,7 @@ extern int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
>> extern void ext4_htree_free_dir_info(struct dir_private_info *p);
>>
>> /* fsync.c */
>> -extern int ext4_sync_file(struct file *, int);
>> +extern int ext4_sync_file(struct file *, loff_t, loff_t, int);
>> extern int ext4_flush_completed_IO(struct inode *);
>>
>> /* hash.c */
>> diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
>> index ce66d2f..afab2e4 100644
>> --- a/fs/ext4/fsync.c
>> +++ b/fs/ext4/fsync.c
>> @@ -165,7 +165,7 @@ static int ext4_sync_parent(struct inode *inode)
>> * i_mutex lock is held when entering and exiting this function
>> */
>>
>> -int ext4_sync_file(struct file *file, int datasync)
>> +int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
>> {
>> struct inode *inode = file->f_mapping->host;
>> struct ext4_inode_info *ei = EXT4_I(inode);
>> @@ -178,15 +178,22 @@ int ext4_sync_file(struct file *file, int datasync)
>>
>> trace_ext4_sync_file_enter(file, datasync);
>>
>> + ret = filemap_write_and_wait_range(inode->i_mapping, start, end);
>> + if (ret)
>> + return ret;
>> + mutex_lock(&inode->i_mutex);
>> +
>> if (inode->i_sb->s_flags & MS_RDONLY)
>> - return 0;
>> + goto out;
>>
>> ret = ext4_flush_completed_IO(inode);
>> if (ret < 0)
>> goto out;
>>
>> if (!journal) {
>> - ret = generic_file_fsync(file, datasync);
>> + if (inode->i_state & I_DIRTY ||
>> + (datasync && (inode->i_state & I_DIRTY_DATASYNC)))
>> + ret = sync_inode_metadata(inode, 1);
> Umm, how did you get to this change? It is actually wrong - ext4 in
> nojournal mode really seems to need to call generic_file_fsync() (so that
> sync_buffers_list() is called).
>
Ah oops sorry about that, I will fix that, I was trying to be clever and
encapsulate generic_file_fsync()'s behavior into here but I missed the
sync_mapping_buffers() call. Thanks,
Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
