Patchwork [2/2] ext4: fix I/O error when unmounting an ro file system

login
register
mail settings
Submitter Theodore Ts'o
Date Oct. 25, 2012, 8:01 p.m.
Message ID <1351195261-31258-3-git-send-email-tytso@mit.edu>
Download mbox | patch
Permalink /patch/194303/
State Rejected
Headers show

Comments

Theodore Ts'o - Oct. 25, 2012, 8:01 p.m.
This comment fixes commit eeecef0af5ea in a different way.  The
original bug was that the following sequence:

# truncate --size=1g fsfile
# mkfs.ext4 -F fsfile
# mount -o loop,ro fsfile /mnt
# umount /mnt
# dmesg | tail

results in an IO error when unmounting the RO filesystem:

[  318.020828] Buffer I/O error on device loop1, logical block 196608
[  318.027024] lost page write due to I/O error on loop1
[  318.032088] JBD2: Error -5 detected when updating journal superblock for l

And this was a regression introduced by commit 24bcc89c7e7c: "jbd2:
split updating of journal superblock and marking journal empty".

Here, we fix this bug by explicitly checking to see if the journal's
block device is read-only.  If it is read-only, we then do a sanity
check to make sure that sb->s_start is zero.  If it is non-zero, that
implies that one or more transactions has been committed to the
journal, which is a "can't happen" scenario if the block device is
read-only.  So we will print a warning message and abort the journal
in that case.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: Nix <nix@esperi.org.uk>
Cc: Toralf Förster <toralf.foerster@gmx.de>
---
 fs/jbd2/journal.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Patch

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index bd23f2e..dcea89f 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1357,6 +1357,24 @@  static void jbd2_mark_journal_empty(journal_t *journal)
 	jbd_debug(1, "JBD2: Marking journal as empty (seq %d)\n",
 		  journal->j_tail_sequence);
 
+	if (bdev_read_only(journal->j_dev)) {
+		__be32 s_start = sb->s_start;
+
+		read_unlock(&journal->j_state_lock);
+		if (s_start != 0) {
+			/*
+			 * should never happen, but aborting the
+			 * journal is nicer than a BUG_ON
+			 */
+			printk(KERN_ERR
+	"JBD2: trying to empty non-empty journal for r/o device %s\n",
+			       journal->j_devname);
+			WARN_ON(1);
+			jbd2_journal_abort(journal, -EROFS);
+		}
+		return;
+	}
+
 	sb->s_sequence = cpu_to_be32(journal->j_tail_sequence);
 	sb->s_start    = cpu_to_be32(0);
 	read_unlock(&journal->j_state_lock);