Patchwork jbd2: Fix oops in jbd2_journal_put_journal_head()

login
register
mail settings
Submitter Jan Kara
Date May 13, 2013, 1:31 p.m.
Message ID <1368451916-16785-1-git-send-email-jack@suse.cz>
Download mbox | patch
Permalink /patch/243404/
State Accepted
Headers show

Comments

Jan Kara - May 13, 2013, 1:31 p.m.
Commit ae4647fb (jbd2: reduce journal_head size.) introduced a
regression where we occasionally hit panic in
jbd2_journal_put_journal_head() because of wrong b_jcount. The bug is
caused by gcc making 64-bit access to 32-bit bitfield and thus
clobbering b_jcount.

At least for now, those 8 bytes saved in struct journal_head are not
worth the trouble with gcc bitfield handling so revert that part of the
patch.

Reported-by: EUNBONG SONG <eunb.song@samsung.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 include/linux/journal-head.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

  Ted, this should fix the regression reported by EUNBONG SONG. Please apply.
Theodore Ts'o - May 13, 2013, 1:49 p.m.
Thanks, applied.  BTW, I've changed the subject line to be "jbd,jbd2",
since the change was to a common journal-head.h file, and although
this problem was identified with ext4, it would apply to ext3 users as
well.

I also added Tony Luck to the reported by line since he also helped to
identify and bisect the problem.  Thanks to Tony and Eunbong for
reporting and then identifying the suspect commit!

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

Patch

diff --git a/include/linux/journal-head.h b/include/linux/journal-head.h
index 13a3da2..3594c74 100644
--- a/include/linux/journal-head.h
+++ b/include/linux/journal-head.h
@@ -30,15 +30,19 @@  struct journal_head {
 
 	/*
 	 * Journalling list for this buffer [jbd_lock_bh_state()]
+	 * NOTE: We *cannot* combine this with b_modified into a bitfield
+	 * as gcc would then (correctly according to C++11) make 64-bit
+	 * accesses to the bitfield and clobber b_jcount if its update
+	 * races with bitfield modification.
 	 */
-	unsigned b_jlist:4;
+	unsigned b_jlist;
 
 	/*
 	 * This flag signals the buffer has been modified by
 	 * the currently running transaction
 	 * [jbd_lock_bh_state()]
 	 */
-	unsigned b_modified:1;
+	unsigned b_modified;
 
 	/*
 	 * Copy of the buffer data frozen for writing to the log.