[PATCH] Fix reiserfs latencies caused by data=ordered

[PATCH] Fix reiserfs latencies caused by data=ordered

[Chris Mason]

From: Chris Maosn <mason@suse.com>
Subject: Fix reiserfs latencies caused by data=ordered

ReiserFS does periodic cleanup of old transactions in order to limit the
length of time a journal replay may take after a crash.  Sometimes, writing
metadata from an old (already committed) transaction may require committing
a newer transaction, which also requires writing all data=ordered buffers.
This can cause very long stalls on journal_begin.

This patch makes sure new transactions will not need to be committed before
trying a periodic reclaim of an old transaction.  It is low risk because
if a bad decision is made, it just means a slightly longer journal
replay after a crash.

Signed-off-by: Chris Mason <mason@suse.com>

--- a/fs/reiserfs/journal.c	Fri Apr 14 13:10:47 2006 -0400
+++ b/fs/reiserfs/journal.c	Sun May 07 22:07:38 2006 -0400
@@ -1189,6 +1189,21 @@ static struct reiserfs_journal_list *fin
 	return NULL;
 }
 
+static int newer_jl_done(struct reiserfs_journal_cnode *cn)
+{
+	struct super_block *sb = cn->sb;
+	b_blocknr_t blocknr = cn->blocknr;
+
+	cn = cn->hprev;
+	while (cn) {
+		if (cn->sb == sb && cn->blocknr == blocknr && cn->jlist &&
+		    atomic_read(&cn->jlist->j_commit_left) != 0)
+				    return 0;
+		cn = cn->hprev;
+	}
+	return 1;
+}
+
 static void remove_journal_hash(struct super_block *,
 				struct reiserfs_journal_cnode **,
 				struct reiserfs_journal_list *, unsigned long,
@@ -1605,6 +1620,31 @@ static int flush_journal_list(struct sup
 		up(&journal->j_flush_sem);
 	put_fs_excl();
 	return err;
+}
+
+static int test_transaction(struct super_block *s,
+                            struct reiserfs_journal_list *jl)
+{
+	struct reiserfs_journal_cnode *cn;
+
+	if (jl->j_len == 0 || atomic_read(&jl->j_nonzerolen) == 0)
+		return 1;
+
+	cn = jl->j_realblock;
+	while (cn) {
+		/* if the blocknr == 0, this has been cleared from the hash,
+		 ** skip it
+		 */
+		if (cn->blocknr == 0) {
+			goto next;
+		}
+		if (cn->bh && !newer_jl_done(cn))
+			return 0;
+	      next:
+		cn = cn->next;
+		cond_resched();
+	}
+	return 0;
 }
 
 static int write_one_transaction(struct super_block *s,
@@ -3436,16 +3476,6 @@ static void flush_async_commits(void *p)
 		flush_commit_list(p_s_sb, jl, 1);
 	}
 	unlock_kernel();
-	/*
-	 * this is a little racey, but there's no harm in missing
-	 * the filemap_fdata_write
-	 */
-	if (!atomic_read(&journal->j_async_throttle)
-	    && !reiserfs_is_journal_aborted(journal)) {
-		atomic_inc(&journal->j_async_throttle);
-		filemap_fdatawrite(p_s_sb->s_bdev->bd_inode->i_mapping);
-		atomic_dec(&journal->j_async_throttle);
-	}
 }
 
 /*
@@ -3847,7 +3877,9 @@ static void flush_old_journal_lists(stru
 		entry = journal->j_journal_list.next;
 		jl = JOURNAL_LIST_ENTRY(entry);
 		/* this check should always be run, to send old lists to disk */
-		if (jl->j_timestamp < (now - (JOURNAL_MAX_TRANS_AGE * 4))) {
+		if (jl->j_timestamp < (now - (JOURNAL_MAX_TRANS_AGE * 4)) &&
+		    atomic_read(&jl->j_commit_left) == 0 &&
+		    test_transaction(s, jl)) {
 			flush_used_journal_lists(s, jl);
 		} else {
 			break;
-- 

Re: [PATCH] Fix reiserfs latencies caused by data=ordered

[Andrew Morton]

On Fri, 4 Aug 2006 10:07:08 -0400
Chris Mason <mason@suse.com> wrote:

> ReiserFS does periodic cleanup of old transactions in order to limit the
> length of time a journal replay may take after a crash.  Sometimes, writing
> metadata from an old (already committed) transaction may require committing
> a newer transaction, which also requires writing all data=ordered buffers.
> This can cause very long stalls on journal_begin.
> 
> This patch makes sure new transactions will not need to be committed before
> trying a periodic reclaim of an old transaction.  It is low risk because
> if a bad decision is made, it just means a slightly longer journal
> replay after a crash.

So I'm thinking that these:

i_mutex-does-not-need-to-be-locked-in-reiserfs_delete_inode.patch
fix-reiserfs-lock-inversion-of-bkl-vs-inode-semaphore.patch (akpm modified)
reiserfs_write_full_page-should-not-get_block-past-eof.patch

are 2.6.18 material.  What are your thoughts on that?

Re: [PATCH] Fix reiserfs latencies caused by data=ordered

[Chris Mason]

On Fri, Aug 04, 2006 at 04:01:22PM -0700, Andrew Morton wrote:
> On Fri, 4 Aug 2006 10:07:08 -0400
> Chris Mason <mason@suse.com> wrote:
> 
> > ReiserFS does periodic cleanup of old transactions in order to limit the
> > length of time a journal replay may take after a crash.  Sometimes, writing
> > metadata from an old (already committed) transaction may require committing
> > a newer transaction, which also requires writing all data=ordered buffers.
> > This can cause very long stalls on journal_begin.
> > 
> > This patch makes sure new transactions will not need to be committed before
> > trying a periodic reclaim of an old transaction.  It is low risk because
> > if a bad decision is made, it just means a slightly longer journal
> > replay after a crash.
> 
> So I'm thinking that these:
> 
> i_mutex-does-not-need-to-be-locked-in-reiserfs_delete_inode.patch
> fix-reiserfs-lock-inversion-of-bkl-vs-inode-semaphore.patch (akpm modified)
> reiserfs_write_full_page-should-not-get_block-past-eof.patch
> 
> are 2.6.18 material.  What are your thoughts on that?

Ack from me.

-chris