From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 09/15] jbd2: replace barriers with explicit flush / FUA usage Date: Wed, 18 Aug 2010 16:03:08 +0200 Message-ID: <20100818140308.GD4680@quack.suse.cz> References: <20100818093432.646633424@bombadil.infradead.org> <20100818093501.353545700@bombadil.infradead.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="OXfL5xGRrasGEqWY" Cc: tj@kernel.org, chris.mason@oracle.com, swhiteho@redhat.com, konishi.ryusuke@lab.ntt.co.jp, tytso@mit.edu, jack@suse.cz, hirofumi@mail.parknet.co.jp, mfasheh@suse.com, joel.becker@oracle.com, hughd@google.com, linux-fsdevel@vger.kernel.org To: Christoph Hellwig Return-path: Received: from cantor2.suse.de ([195.135.220.15]:48938 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752827Ab0HRODv (ORCPT ); Wed, 18 Aug 2010 10:03:51 -0400 Content-Disposition: inline In-Reply-To: <20100818093501.353545700@bombadil.infradead.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --OXfL5xGRrasGEqWY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 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 > > 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 SUSE Labs, CR --OXfL5xGRrasGEqWY Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="0001-jbd2-Modify-ASYNC_COMMIT-code-to-not-rely-on-queue-d.patch" >>From 21a76eb2010faf0c4507a238d2d110bac9842146 Mon Sep 17 00:00:00 2001 From: Jan Kara 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 --- 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 --OXfL5xGRrasGEqWY--