We're seeing heavy contention against j_list_lock on 8-way in
do_get_write_access().
We actually don't need j_list_lock in there except for one little case - the
per-bh jbd_lock_bh_state() is sufficient to protect this buffer's internal
state.
On some nice quick LVM array Ram Pai measured an overall 3x speedup from this
patch:
the script took the following time on 265mm1
real 0m57.504s
user 0m0.400s
sys 7m29.867s
and with the 2patches it took
real 0m19.983s
user 0m0.438s
sys 1m55.896s
---
25-akpm/fs/jbd/transaction.c | 58 +++++++++++++++++++++----------------------
fs/jbd/commit.c | 0
2 files changed, 29 insertions(+), 29 deletions(-)
diff -puN fs/jbd/transaction.c~jbd-do_get_write_access-lock-contention-reduction fs/jbd/transaction.c
--- 25/fs/jbd/transaction.c~jbd-do_get_write_access-lock-contention-reduction 2004-04-06 00:00:38.041191696 -0700
+++ 25-akpm/fs/jbd/transaction.c 2004-04-06 00:01:50.880118496 -0700
@@ -522,7 +522,6 @@ static void jbd_unexpected_dirty_buffer(
* part of the transaction, that is).
*
*/
-
static int
do_get_write_access(handle_t *handle, struct journal_head *jh,
int force_copy, int *credits)
@@ -550,7 +549,6 @@ repeat:
lock_buffer(bh);
jbd_lock_bh_state(bh);
- spin_lock(&journal->j_list_lock);
/* We now hold the buffer lock so it is safe to query the buffer
* state. Is the buffer dirty?
@@ -566,9 +564,10 @@ repeat:
* the buffer dirtied, ugh.) */
if (buffer_dirty(bh)) {
- /* First question: is this buffer already part of the
- * current transaction or the existing committing
- * transaction? */
+ /*
+ * First question: is this buffer already part of the current
+ * transaction or the existing committing transaction?
+ */
if (jh->b_transaction) {
J_ASSERT_JH(jh,
jh->b_transaction == transaction ||
@@ -586,22 +585,23 @@ repeat:
error = -EROFS;
if (is_handle_aborted(handle)) {
- spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh);
- goto out_unlocked;
+ goto out;
}
error = 0;
- /* The buffer is already part of this transaction if
- * b_transaction or b_next_transaction points to it. */
-
+ /*
+ * The buffer is already part of this transaction if b_transaction or
+ * b_next_transaction points to it
+ */
if (jh->b_transaction == transaction ||
jh->b_next_transaction == transaction)
- goto done_locked;
-
- /* If there is already a copy-out version of this buffer, then
- * we don't need to make another one. */
+ goto done;
+ /*
+ * If there is already a copy-out version of this buffer, then we don't
+ * need to make another one
+ */
if (jh->b_frozen_data) {
JBUFFER_TRACE(jh, "has frozen data");
J_ASSERT_JH(jh, jh->b_next_transaction == NULL);
@@ -611,7 +611,7 @@ repeat:
handle->h_buffer_credits--;
if (credits)
(*credits)++;
- goto done_locked;
+ goto done;
}
/* Is there data here we need to preserve? */
@@ -635,7 +635,6 @@ repeat:
wait_queue_head_t *wqh;
JBUFFER_TRACE(jh, "on shadow: sleep");
- spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh);
/* commit wakes up all shadow buffers after IO */
wqh = bh_waitq_head(jh2bh(jh));
@@ -661,7 +660,6 @@ repeat:
JBUFFER_TRACE(jh, "generate frozen data");
if (!frozen_buffer) {
JBUFFER_TRACE(jh, "allocate memory for buffer");
- spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh);
frozen_buffer = jbd_kmalloc(jh2bh(jh)->b_size,
GFP_NOFS);
@@ -672,12 +670,10 @@ repeat:
JBUFFER_TRACE(jh, "oom!");
error = -ENOMEM;
jbd_lock_bh_state(bh);
- spin_lock(&journal->j_list_lock);
- goto done_locked;
+ goto done;
}
goto repeat;
}
-
jh->b_frozen_data = frozen_buffer;
frozen_buffer = NULL;
need_copy = 1;
@@ -690,20 +686,22 @@ repeat:
if (credits)
(*credits)++;
- /* Finally, if the buffer is not journaled right now, we need to
- * make sure it doesn't get written to disk before the caller
- * actually commits the new data. */
-
+ /*
+ * Finally, if the buffer is not journaled right now, we need to make
+ * sure it doesn't get written to disk before the caller actually
+ * commits the new data
+ */
if (!jh->b_transaction) {
JBUFFER_TRACE(jh, "no transaction");
J_ASSERT_JH(jh, !jh->b_next_transaction);
jh->b_transaction = transaction;
JBUFFER_TRACE(jh, "file as BJ_Reserved");
+ spin_lock(&journal->j_list_lock);
__journal_file_buffer(jh, transaction, BJ_Reserved);
+ spin_unlock(&journal->j_list_lock);
}
-done_locked:
- spin_unlock(&journal->j_list_lock);
+done:
if (need_copy) {
struct page *page;
int offset;
@@ -719,11 +717,13 @@ done_locked:
}
jbd_unlock_bh_state(bh);
- /* If we are about to journal a buffer, then any revoke pending
- on it is no longer valid. */
+ /*
+ * If we are about to journal a buffer, then any revoke pending on it is
+ * no longer valid
+ */
journal_cancel_revoke(handle, jh);
-out_unlocked:
+out:
if (frozen_buffer)
kfree(frozen_buffer);
diff -puN fs/jbd/commit.c~jbd-do_get_write_access-lock-contention-reduction fs/jbd/commit.c
_