All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: tytso@mit.edu
Cc: Edward Goggin <egoggin@vmware.com>,
	linux-ext4@vger.kernel.org, Jan Kara <jack@suse.cz>
Subject: [PATCH 2/2] ext4: Fix waiting and sending of a barrier in ext4_sync_file()
Date: Tue, 17 May 2011 12:28:15 +0200	[thread overview]
Message-ID: <1305628095-27843-3-git-send-email-jack@suse.cz> (raw)
In-Reply-To: <1305628095-27843-1-git-send-email-jack@suse.cz>

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(-)

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);
-- 
1.7.1


  parent reply	other threads:[~2011-05-17 10:28 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-17 10:28 [PATCH 0/2] Fix sending of a barrier and transaction waiting in ext4_sync_file() Jan Kara
2011-05-17 10:28 ` [PATCH 1/2] jbd2: Provide function to check whether transaction will issue data barrier Jan Kara
2011-05-17 10:28 ` Jan Kara [this message]
2011-05-22 21:12   ` [PATCH 2/2] ext4: Fix waiting and sending of a barrier in ext4_sync_file() Ted Ts'o
2011-05-22 21:13     ` [PATCH 1/2] jbd2: Add function jbd2_trans_will_send_data_barrier() Theodore Ts'o
2011-05-22 21:13     ` [PATCH 2/2] ext4: Fix waiting and sending of a barrier in ext4_sync_file() Theodore Ts'o
2011-05-23 17:17       ` Jan Kara
2011-05-23 19:28         ` Ted Ts'o
2011-05-23 20:25           ` Jan Kara
2011-05-18 17:43 ` [PATCH 0/2] Fix sending of a barrier and transaction waiting " Darrick J. Wong
2011-05-19 10:51   ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1305628095-27843-3-git-send-email-jack@suse.cz \
    --to=jack@suse.cz \
    --cc=egoggin@vmware.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.