mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Theodore Tso <tytso@mit.edu>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@infradead.org>,
	Jens Axboe <jens.axboe@oracle.com>,
	linux-kernel@vger.kernel.org, Alan Cox <alan@lxorguk.ukuu.org.uk>,
	linux-ext4@vger.kernel.org
Subject: Re: [PATCH] Give kjournald a IOPRIO_CLASS_RT io priority
Date: Wed, 8 Oct 2008 23:00:54 -0400	[thread overview]
Message-ID: <20081009030054.GD17512@mit.edu> (raw)
In-Reply-To: <20081002222438.8f9f90a2.akpm@linux-foundation.org>

On Thu, Oct 02, 2008 at 10:24:38PM -0700, Andrew Morton wrote:
> 
> Mount a junk partition with `-oakpm' and run some benchmarks.  If the
> results are "wow" then it's worth spending time on.  If the results are
> "meh" then we can not bother..
> 

I've ported the patch to the ext4 filesystem, and dropped it into the
unstable portion of the ext4 patch queue.  If we can get someone (hi,
Ric!) to run fs_mark with and without -o akpm_lock_hack, I suspect we
will find that it makes quite a large difference on that particular
benchmark, since it is fsync-heavy to force a large number of
transaction, and the creation of the inodes should cause multiple
blocks that will be entangled between the current and committing
transactions.

	      	     	     	      	      	 - Ted

ext4: akpm's locking hack to fix locking delays

This is a port of the following patch from Andrew Morton to ext4:

	http://lkml.org/lkml/2008/10/3/22

This fixes a major contention problem in do_get_write_access() when a
buffer is modified in both the current and committing transaction.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: akpm@linux-foundation.org
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index f46a513..23822fb 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -540,6 +540,7 @@ do {									       \
 #define EXT4_MOUNT_JOURNAL_ASYNC_COMMIT	0x1000000 /* Journal Async Commit */
 #define EXT4_MOUNT_I_VERSION            0x2000000 /* i_version support */
 #define EXT4_MOUNT_DELALLOC		0x8000000 /* Delalloc support */
+#define EXT4_MOUNT_AKPM_LOCK_HACK	0x10000000 /* akpm lock hack */
 /* Compatibility, for having both ext2_fs.h and ext4_fs.h included at once */
 #ifndef _LINUX_EXT2_FS_H
 #define clear_opt(o, opt)		o &= ~EXT4_MOUNT_##opt
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 67ebefb..f4e7157 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -752,6 +752,8 @@ static int ext4_show_options(struct seq_file *seq, struct vfsmount *vfs)
 		seq_puts(seq, ",journal_async_commit");
 	if (test_opt(sb, NOBH))
 		seq_puts(seq, ",nobh");
+	if (test_opt(sb, AKPM_LOCK_HACK))
+		seq_puts(seq, ",akpm_lock_hack");
 	if (!test_opt(sb, EXTENTS))
 		seq_puts(seq, ",noextents");
 	if (test_opt(sb, I_VERSION))
@@ -911,7 +913,7 @@ enum {
 	Opt_ignore, Opt_barrier, Opt_err, Opt_resize, Opt_usrquota,
 	Opt_grpquota, Opt_extents, Opt_noextents, Opt_i_version,
 	Opt_mballoc, Opt_nomballoc, Opt_stripe, Opt_delalloc, Opt_nodelalloc,
-	Opt_inode_readahead_blks
+	Opt_inode_readahead_blks, Opt_akpm_lock_hack,
 };
 
 static match_table_t tokens = {
@@ -973,6 +975,7 @@ static match_table_t tokens = {
 	{Opt_delalloc, "delalloc"},
 	{Opt_nodelalloc, "nodelalloc"},
 	{Opt_inode_readahead_blks, "inode_readahead_blks=%u"},
+	{Opt_akpm_lock_hack, "akpm_lock_hack"},
 	{Opt_err, NULL},
 };
 
@@ -1382,6 +1385,9 @@ set_qf_format:
 				return 0;
 			sbi->s_inode_readahead_blks = option;
 			break;
+		case Opt_akpm_lock_hack:
+			set_opt(sbi->s_mount_opt, AKPM_LOCK_HACK);
+			break;
 		default:
 			printk(KERN_ERR
 			       "EXT4-fs: Unrecognized mount option \"%s\" "
@@ -2534,6 +2540,10 @@ static void ext4_init_journal_params(struct super_block *sb, journal_t *journal)
 		journal->j_flags |= JBD2_BARRIER;
 	else
 		journal->j_flags &= ~JBD2_BARRIER;
+	if (test_opt(sb, AKPM_LOCK_HACK))
+		journal->j_flags |= JBD2_LOCK_HACK;
+	else
+		journal->j_flags &= ~JBD2_LOCK_HACK;
 	spin_unlock(&journal->j_state_lock);
 }
 
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index e5d5405..32c288a 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -546,6 +546,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 	int error;
 	char *frozen_buffer = NULL;
 	int need_copy = 0;
+	int locked = 0;
 
 	if (is_handle_aborted(handle))
 		return -EROFS;
@@ -561,7 +562,13 @@ repeat:
 
 	/* @@@ Need to check for errors here at some point. */
 
-	lock_buffer(bh);
+	if (journal->j_flags & JBD2_LOCK_HACK) {
+		if (trylock_buffer(bh))
+			locked = 1;	/* lolz */
+	} else {
+		lock_buffer(bh);
+		locked = 1;
+	}
 	jbd_lock_bh_state(bh);
 
 	/* We now hold the buffer lock so it is safe to query the buffer
@@ -600,7 +607,8 @@ repeat:
 		jbd_unexpected_dirty_buffer(jh);
 	}
 
-	unlock_buffer(bh);
+	if (locked)
+		unlock_buffer(bh);
 
 	error = -EROFS;
 	if (is_handle_aborted(handle)) {
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 66c3499..c614dae 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -967,6 +967,7 @@ struct journal_s
 #define JBD2_FLUSHED	0x008	/* The journal superblock has been flushed */
 #define JBD2_LOADED	0x010	/* The journal superblock has been loaded */
 #define JBD2_BARRIER	0x020	/* Use IDE barriers */
+#define JBD2_LOCK_HACK	0x040	/* akpm's locking hack */
 
 /*
  * Function declarations for the journaling transaction and buffer

  parent reply	other threads:[~2008-10-09  3:01 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-02  3:00 Arjan van de Ven
2008-10-02  4:56 ` Andrew Morton
2008-10-02  6:27   ` Jens Axboe
2008-10-02  6:55     ` Andrew Morton
2008-10-02  7:45       ` Jens Axboe
2008-10-02  8:03         ` Andrew Morton
2008-10-02  8:22           ` Jens Axboe
2008-10-02  8:43             ` Andrew Morton
2008-10-02  8:46               ` Jens Axboe
2008-10-02 12:04           ` Theodore Tso
2008-10-02 13:16             ` Arjan van de Ven
2008-10-02 13:46               ` Theodore Tso
2008-10-02 14:33                 ` Arjan van de Ven
2008-10-04 14:12                   ` Theodore Tso
2008-10-04 17:14                     ` Joseph Fannin
2008-10-04 21:27                       ` Theodore Tso
2008-10-02 13:12       ` Arjan van de Ven
2008-10-02 20:24         ` Andrew Morton
2008-10-03  4:01           ` Arjan van de Ven
2008-10-03  4:23             ` Arjan van de Ven
2008-10-03  4:40               ` Andrew Morton
2008-10-03  4:43                 ` Arjan van de Ven
2008-10-03  4:50                   ` Andrew Morton
2008-10-03  5:00                     ` Arjan van de Ven
2008-10-03  5:24                       ` Andrew Morton
2008-10-03 17:21                         ` Arjan van de Ven
2008-10-09  3:00                         ` Theodore Tso [this message]
2008-10-09  3:38                           ` Andrew Morton
2008-10-03  4:45                 ` Arjan van de Ven
2008-10-02  6:57   ` Andi Kleen
2008-10-02  7:55     ` Jens Axboe
2008-10-02  9:33       ` Dave Chinner
2008-10-02  9:45         ` Jens Axboe
2008-10-02 13:14           ` Arjan van de Ven
2008-10-02 13:27             ` Jens Axboe
2008-10-02 13:36               ` Arjan van de Ven
2008-10-02 13:47                 ` Jens Axboe
2008-10-02 14:26                   ` Arjan van de Ven
2008-10-02 16:42                     ` Jens Axboe
2008-10-02 19:04           ` Arjan van de Ven
2008-10-02 19:22             ` Jens Axboe
2008-10-02 21:37               ` Andrew Morton
2008-10-02 23:58                 ` Dave Chinner
2008-10-03  0:06                   ` Andrew Morton
2008-10-03  0:20                     ` Andrew Morton
2008-10-02 13:05   ` Arjan van de Ven
2008-10-02 17:11     ` Jens Axboe
     [not found] <bimJN-4cO-5@gated-at.bofh.it>
     [not found] ` <biosl-6bq-9@gated-at.bofh.it>
     [not found]   ` <biqkw-aK-3@gated-at.bofh.it>
     [not found]     ` <birgx-1pQ-9@gated-at.bofh.it>
     [not found]       ` <bisPe-3xx-9@gated-at.bofh.it>
     [not found]         ` <bisYW-3HQ-13@gated-at.bofh.it>
2008-10-02 15:32           ` Bodo Eggert
2008-10-02 23:34             ` Dave Chinner
2008-10-04  7:45               ` Aaron Carroll
2008-10-06  3:18                 ` Dave Chinner
2008-10-07 18:06                   ` Jens Axboe
2008-10-07 22:22                     ` Dave Chinner
2008-10-09  8:48                       ` Jens Axboe
  -- strict thread matches above, loose matches on Subject: below --
2007-10-15 17:46 [patch] " Arjan van de Ven
2007-10-15 18:47 ` Andrew Morton
2007-10-15 19:28   ` Jens Axboe
2007-10-22  9:10     ` Ingo Molnar
2007-10-22  9:23       ` Andrew Morton
2007-10-22  9:27         ` Ingo Molnar
2007-10-22  9:40         ` Ingo Molnar
2007-10-22  9:49           ` Andrew Morton
2007-10-15 20:13   ` Rik van Riel
2007-10-15 21:12     ` Andrew Morton
     [not found]       ` <473B18BA.5000709@hp.com>
2007-11-14 17:14         ` Andrew Morton
2007-11-14 17:18           ` Ingo Molnar
2007-11-14 17:51             ` Arjan van de Ven
2007-11-14 18:55               ` Ingo Molnar
2007-11-14 19:43               ` Alan D. Brunelle
2007-11-14 19:24           ` Alan D. Brunelle
2007-11-14 19:50             ` Arjan van de Ven
2007-11-14 19:56             ` Alan D. Brunelle
2007-11-16 16:25           ` Alan D. Brunelle
2007-11-16 16:40             ` Alan D. Brunelle
2007-11-16 18:35             ` Ray Lee
2007-11-16 18:39               ` Alan D. Brunelle

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=20081009030054.GD17512@mit.edu \
    --to=tytso@mit.edu \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=arjan@infradead.org \
    --cc=jens.axboe@oracle.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome