On Wed 18-08-10 05:29:17, Christoph Hellwig wrote:
> Switch to the WRITE_FLUSH_FUA flag for journal commits and remove the
> EOPNOTSUPP detection for barriers.
Well, in the context of just this patch series, the patch is OK so you
can add
Acked-by: Jan Kara <jack@xxxxxxx>
But as soon as blkdev_issue_flush() stops draining the queue, commit when
JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT is set breaks. Do you plan to fix
that separately? Actually, attached is a patch which should fix
ASYNC_COMMIT code... The patch is totally untested, I just wrote it
because it's simpler than explaining what needs to be done ;).
Honza
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>
> Index: linux-2.6/fs/jbd2/commit.c
> ===================================================================
> --- linux-2.6.orig/fs/jbd2/commit.c 2010-08-17 16:52:37.111011684 +0200
> +++ linux-2.6/fs/jbd2/commit.c 2010-08-17 16:53:50.531029420 +0200
> @@ -134,25 +134,10 @@ static int journal_submit_commit_record(
>
> if (journal->j_flags & JBD2_BARRIER &&
> !JBD2_HAS_INCOMPAT_FEATURE(journal,
> - JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT)) {
> - ret = submit_bh(WRITE_SYNC_PLUG | WRITE_BARRIER, bh);
> - if (ret == -EOPNOTSUPP) {
> - printk(KERN_WARNING
> - "JBD: barrier-based sync failed on %s - "
> - "disabling barriers\n", journal->j_devname);
> - spin_lock(&journal->j_state_lock);
> - journal->j_flags &= ~JBD2_BARRIER;
> - spin_unlock(&journal->j_state_lock);
> -
> - /* And try again, without the barrier */
> - lock_buffer(bh);
> - set_buffer_uptodate(bh);
> - clear_buffer_dirty(bh);
> - ret = submit_bh(WRITE_SYNC_PLUG, bh);
> - }
> - } else {
> + JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT))
> + ret = submit_bh(WRITE_SYNC_PLUG | WRITE_FLUSH_FUA, bh);
> + else
> ret = submit_bh(WRITE_SYNC_PLUG, bh);
> - }
>
> *cbh = bh;
> return ret;
> @@ -167,29 +152,8 @@ static int journal_wait_on_commit_record
> {
> int ret = 0;
>
> -retry:
> clear_buffer_dirty(bh);
> wait_on_buffer(bh);
> - if (buffer_eopnotsupp(bh) && (journal->j_flags & JBD2_BARRIER)) {
> - printk(KERN_WARNING
> - "JBD2: wait_on_commit_record: sync failed on %s - "
> - "disabling barriers\n", journal->j_devname);
> - spin_lock(&journal->j_state_lock);
> - journal->j_flags &= ~JBD2_BARRIER;
> - spin_unlock(&journal->j_state_lock);
> -
> - lock_buffer(bh);
> - clear_buffer_dirty(bh);
> - set_buffer_uptodate(bh);
> - bh->b_end_io = journal_end_buffer_io_sync;
> -
> - ret = submit_bh(WRITE_SYNC_PLUG, bh);
> - if (ret) {
> - unlock_buffer(bh);
> - return ret;
> - }
> - goto retry;
> - }
>
> if (unlikely(!buffer_uptodate(bh)))
> ret = -EIO;
>
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
>From 21a76eb2010faf0c4507a238d2d110bac9842146 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@xxxxxxx>
Date: Wed, 18 Aug 2010 15:56:56 +0200
Subject: [PATCH] jbd2: Modify ASYNC_COMMIT code to not rely on queue draining on barrier
Currently JBD2 relies blkdev_issue_flush() draining the queue when ASYNC_COMMIT
feature is set. This property is going away so make JBD2 wait for buffers it
needs on its own before submitting the cache flush.
Signed-off-by: Jan Kara <jack@xxxxxxx>
---
fs/jbd2/commit.c | 28 +++++++++++++++-------------
1 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 6a25e51..4c52f78 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -667,6 +667,15 @@ start_journal_io:
}
}
+ err = journal_finish_inode_data_buffers(journal, commit_transaction);
+ if (err) {
+ printk(KERN_WARNING
+ "JBD2: Detected IO errors while flushing file data "
+ "on %s\n", journal->j_devname);
+ if (journal->j_flags & JBD2_ABORT_ON_SYNCDATA_ERR)
+ jbd2_journal_abort(journal, err);
+ err = 0;
+ }
/*
* If the journal is not located on the file system device,
* then we must flush the file system device before we issue
@@ -685,19 +694,6 @@ start_journal_io:
&cbh, crc32_sum);
if (err)
__jbd2_journal_abort_hard(journal);
- if (journal->j_flags & JBD2_BARRIER)
- blkdev_issue_flush(journal->j_dev, GFP_KERNEL, NULL,
- BLKDEV_IFL_WAIT);
- }
-
- err = journal_finish_inode_data_buffers(journal, commit_transaction);
- if (err) {
- printk(KERN_WARNING
- "JBD2: Detected IO errors while flushing file data "
- "on %s\n", journal->j_devname);
- if (journal->j_flags & JBD2_ABORT_ON_SYNCDATA_ERR)
- jbd2_journal_abort(journal, err);
- err = 0;
}
/* Lo and behold: we have just managed to send a transaction to
@@ -811,6 +807,12 @@ wait_for_iobuf:
}
if (!err && !is_journal_aborted(journal))
err = journal_wait_on_commit_record(journal, cbh);
+ if (JBD2_HAS_INCOMPAT_FEATURE(journal,
+ JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT) &&
+ journal->j_flags & JBD2_BARRIER) {
+ blkdev_issue_flush(journal->j_dev, GFP_KERNEL, NULL,
+ BLKDEV_IFL_WAIT);
+ }
if (err)
jbd2_journal_abort(journal, err);
--
1.6.4.2
