diff mbox

[2/2] ext4: Fix waiting and sending of a barrier in ext4_sync_file()

Message ID 1305628095-27843-3-git-send-email-jack@suse.cz
State Superseded, archived
Headers show

Commit Message

Jan Kara May 17, 2011, 10:28 a.m. UTC
jbd2_log_start_commit() returns 1 only when we really start a transaction.  But
we also need to wait for a transaction when the commit is already running.  Fix
this problem by waiting for transaction commit unconditionally (which is just a
quick check if the transaction is already committed).

Also we have to be more careful with sending of a barrier because when
transaction is being committed in parallel to ext4_sync_file() running, we
cannot be sure that the barrier the journalling code sends happens after we
wrote all the data for fsync (note that not every data writeout needs to
trigger metadata changes thus commit of some metadata changes can be running
while other data is still written out). So use jbd2_will_send_data_barrier()
helper to detect the common cases when we can be sure barrier will be issued by
the commit code and issue the barrier ourselves in the remaining cases. This
code change will cause ext4_sync_file() to unnecessarily issue a barrier in
data=ordered and data=journal mode with external journal but solving this would
require ext4 to mess with jbd2 internals since jbd2 does not know about ext4
data journalling modes.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/fsync.c |   23 +++++++----------------
 1 files changed, 7 insertions(+), 16 deletions(-)

Comments

Theodore Ts'o May 22, 2011, 9:12 p.m. UTC | #1
On Tue, May 17, 2011 at 12:28:15PM +0200, Jan Kara wrote:
> This code change will cause ext4_sync_file() to unnecessarily issue
> a barrier in data=ordered and data=journal mode with external
> journal but solving this would require ext4 to mess with jbd2
> internals since jbd2 does not know about ext4 data journalling
> modes.

I really don't like this.  I think the following two patches should
address this.  Could you take a look please?

-- 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 mbox

Patch

diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
index e9473cb..1c44f1a 100644
--- a/fs/ext4/fsync.c
+++ b/fs/ext4/fsync.c
@@ -172,6 +172,7 @@  int ext4_sync_file(struct file *file, int datasync)
 	journal_t *journal = EXT4_SB(inode->i_sb)->s_journal;
 	int ret;
 	tid_t commit_tid;
+	bool needs_barrier = false;
 
 	J_ASSERT(ext4_journal_current_handle() == NULL);
 
@@ -211,22 +212,12 @@  int ext4_sync_file(struct file *file, int datasync)
 	}
 
 	commit_tid = datasync ? ei->i_datasync_tid : ei->i_sync_tid;
-	if (jbd2_log_start_commit(journal, commit_tid)) {
-		/*
-		 * When the journal is on a different device than the
-		 * fs data disk, we need to issue the barrier in
-		 * writeback mode.  (In ordered mode, the jbd2 layer
-		 * will take care of issuing the barrier.  In
-		 * data=journal, all of the data blocks are written to
-		 * the journal device.)
-		 */
-		if (ext4_should_writeback_data(inode) &&
-		    (journal->j_fs_dev != journal->j_dev) &&
-		    (journal->j_flags & JBD2_BARRIER))
-			blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL,
-					NULL);
-		ret = jbd2_log_wait_commit(journal, commit_tid);
-	} else if (journal->j_flags & JBD2_BARRIER)
+	if (journal->j_flags & JBD2_BARRIER &&
+	    !jbd2_trans_will_send_data_barrier(journal, commit_tid))
+		needs_barrier = true;
+	jbd2_log_start_commit(journal, commit_tid);
+	ret = jbd2_log_wait_commit(journal, commit_tid);
+	if (needs_barrier)
 		blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
  out:
 	trace_ext4_sync_file_exit(inode, ret);