| Message ID | 1046256826.88691438049846580.JavaMail.weblogic@epmlwas02d |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Tue, Jul 28, 2015 at 02:17:26AM +0000, Daeho Jeong wrote: > When fsync() system call is executed, all data of the file should be written on the storage. > But, in a case of that the file is operated in data journal mode, the data doesn't need to be > flushed out and waited for its I/O completion. Moreover, by invoking filemap_fdatawrite() > function on the data journaled file, ext4_sync_file() suffers from an unnecessary delay > which is caused by flushing out and waiting for the I/O completion of a ton of newly-dirtied > pages which were dirtied by the previous commit. No, this patch is incorrect. Data journalling works by having the buffers getting dirtied, and that doesn't happen until we call filemap_fdatawrite_range(). It's true that we don't have to call filemap_write_and_wait_range(), but simply skipping the writeback entirely will cause data writes to get lost after an fsync() if we crash immediately afterwards. I'm guessing you didn't actually test this patch for correctness? - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c index 8850254..e0fabf7 100644 --- a/fs/ext4/fsync.c +++ b/fs/ext4/fsync.c @@ -112,9 +112,6 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync) goto out; } - ret = filemap_write_and_wait_range(inode->i_mapping, start, end); - if (ret) - return ret; /* * data=writeback,ordered: * The caller's filemap_fdatawrite()/wait will sync the data. @@ -125,13 +122,15 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync) * filemap_fdatawrite won't do anything (the buffers are clean). * ext4_force_commit will write the file data into the journal and * will wait on that. - * filemap_fdatawait() will encounter a ton of newly-dirtied pages - * (they were dirtied by commit). But that's OK - the blocks are - * safe in-journal, which is all fsync() needs to ensure. */ if (ext4_should_journal_data(inode)) { ret = ext4_force_commit(inode->i_sb); goto out; + } else { + ret = filemap_write_and_wait_range(inode->i_mapping, + start, end); + if (ret) + return ret; } commit_tid = datasync ? ei->i_datasync_tid : ei->i_sync_tid;
When fsync() system call is executed, all data of the file should be written on the storage. But, in a case of that the file is operated in data journal mode, the data doesn't need to be flushed out and waited for its I/O completion. Moreover, by invoking filemap_fdatawrite() function on the data journaled file, ext4_sync_file() suffers from an unnecessary delay which is caused by flushing out and waiting for the I/O completion of a ton of newly-dirtied pages which were dirtied by the previous commit. Signed-off-by: Daeho Jeong <daeho.jeong@samsung.com> --- fs/ext4/fsync.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) -- 1.7.9.5