linux-ext4 - [PATCH] jbd/2[stable only]: Use WRITE_SYNC_PLUG in journal_commit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 12 Jul 2011 18:43:51 +0800
From:	Tao Ma <tm@....ma>
To:	linux-ext4@...r.kernel.org
Cc:	stable@...nel.org, Jan Kara <jack@...e.cz>,
	Vivek Goyal <vgoyal@...hat.com>,
	Jeff Moyer <jmoyer@...hat.com>,
	Corrado Zoccolo <czoccolo@...il.com>,
	Jens Axboe <jaxboe@...ionio.com>
Subject: [PATCH] jbd/2[stable only]: Use WRITE_SYNC_PLUG in journal_commit_transaction.

From: Tao Ma <boyu.mt@...bao.com>

In commit 749ef9f8423, we use WRITE_SYNC instead of WRITE in
journal_commit_transaction. It causes a much heavy burden for
the disk as now the seqenctial write can't be merged(see the blktrace below).

Given the description of that commit 749ef9f8423, the reason why
we use WRITE_SYNC is that it wants to use REQ_NOIDLE and WRITE_SYNC_PLUG
also has that flag, so use WRITE_SYNC_PLUG instead. From blktrace,
we can get that:

without the patch:
  8,0    6       18     0.016058423  3342  D   W 461101317 + 4 [jbd2/sda11-8]
  8,0    6       19     0.016065473  3342  D   W 461101321 + 4 [jbd2/sda11-8]
  8,0    6       20     0.016070751  3342  D   W 461101325 + 4 [jbd2/sda11-8]
  8,0    6       21     0.016076180  3342  D   W 461101329 + 4 [jbd2/sda11-8]
  8,0    6       22     0.016081255  3342  D   W 461101333 + 4 [jbd2/sda11-8]
  8,0    6       23     0.016085963  3342  D   W 461101337 + 4 [jbd2/sda11-8]
  8,0    6       24     0.016182048     0  C   W 461101317 + 4 [0]
  8,0    6       25     0.016190820     0  C   W 461101325 + 4 [0]
  8,0    6       26     0.016193927     0  C   W 461101321 + 4 [0]
  8,0    6       27     0.016196532     0  C   W 461101333 + 4 [0]
  8,0    6       28     0.016199180     0  C   W 461101337 + 4 [0]
  8,0    6       29     0.016206180     0  C   W 461101329 + 4 [0]

with this patch:
  8,0    4       23     4.320315739  3129  D   W 461101317 + 24 [jbd2/sda11-8]
  8,0    4       24     4.320364518     0  C   W 461101317 + 24 [0]

This only affects .37 and .38 since Jens' new plug patches are included
in .39 and the bug is removed as a side effect. But I think it is needed
anyway for the stable. And RHEL6 needs this also I guess.

Cc: stable@...nel.org # 2.6.37 and 2.6.38
Cc: Jan Kara <jack@...e.cz>
Cc: Vivek Goyal <vgoyal@...hat.com>
Cc: Jeff Moyer <jmoyer@...hat.com>
Cc: Corrado Zoccolo <czoccolo@...il.com>
Cc: Jens Axboe <jaxboe@...ionio.com>
Signed-off-by: Tao Ma <boyu.mt@...bao.com>
---
 fs/jbd/commit.c  |    9 +--------
 fs/jbd2/commit.c |    9 +--------
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/fs/jbd/commit.c b/fs/jbd/commit.c
index 34a4861..6d13df5 100644
--- a/fs/jbd/commit.c
+++ b/fs/jbd/commit.c
@@ -294,7 +294,7 @@ void journal_commit_transaction(journal_t *journal)
 	int first_tag = 0;
 	int tag_flag;
 	int i;
-	int write_op = WRITE_SYNC;
+	int write_op = WRITE_SYNC_PLUG;
 
 	/*
 	 * First job: lock down the current transaction and wait for
@@ -327,13 +327,6 @@ void journal_commit_transaction(journal_t *journal)
 	spin_lock(&journal->j_state_lock);
 	commit_transaction->t_state = T_LOCKED;
 
-	/*
-	 * Use plugged writes here, since we want to submit several before
-	 * we unplug the device. We don't do explicit unplugging in here,
-	 * instead we rely on sync_buffer() doing the unplug for us.
-	 */
-	if (commit_transaction->t_synchronous_commit)
-		write_op = WRITE_SYNC_PLUG;
 	spin_lock(&commit_transaction->t_handle_lock);
 	while (commit_transaction->t_updates) {
 		DEFINE_WAIT(wait);
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index f3ad159..fc3840f 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -329,7 +329,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
 	int tag_bytes = journal_tag_bytes(journal);
 	struct buffer_head *cbh = NULL; /* For transactional checksums */
 	__u32 crc32_sum = ~0;
-	int write_op = WRITE_SYNC;
+	int write_op = WRITE_SYNC_PLUG;
 
 	/*
 	 * First job: lock down the current transaction and wait for
@@ -363,13 +363,6 @@ void jbd2_journal_commit_transaction(journal_t *journal)
 	write_lock(&journal->j_state_lock);
 	commit_transaction->t_state = T_LOCKED;
 
-	/*
-	 * Use plugged writes here, since we want to submit several before
-	 * we unplug the device. We don't do explicit unplugging in here,
-	 * instead we rely on sync_buffer() doing the unplug for us.
-	 */
-	if (commit_transaction->t_synchronous_commit)
-		write_op = WRITE_SYNC_PLUG;
 	trace_jbd2_commit_locking(journal, commit_transaction);
 	stats.run.rs_wait = commit_transaction->t_max_wait;
 	stats.run.rs_locked = jiffies;
-- 
1.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html