How should e2fsck clear s_errno/j_errno on an ro mount?

How should e2fsck clear s_errno/j_errno on an ro mount?

[Eric Sandeen]

I'm looking at a situation where a root filesystem encountered an error and shut down, and therefore the error was stored in the journal.

But for the root fs, it seems that nothing can clear it.

If we do e2fsck -fy on a readonly mounted filesystem, then remount,rw the error persists:

[25124.319387] EXT4-fs warning (device loop3): ext4_clear_journal_err:4281: Filesystem error recorded from previous mount: IO failure
[25124.331140] EXT4-fs warning (device loop3): ext4_clear_journal_err:4282: Marking fs in need of filesystem check.

ad infinitum.

It may be my fever-addled brain this week but I'm having a hard time following how this error is supposed to get set & cleared, especially if e2fsck has modified it while mounted ro.

As soon as I mount rw again, load_superblock() sees the journal superblock has an error set, and copies it back into the journal->j_errno.

After Ted's "e2fsck: correctly propagate error from journal to superblock" in e2fsprogs, at least an unmounted fs gets cleaned up, but I'm not sure what to do to fix this when it's mounted.  

Am I missing something obvious?

Thanks,
-Eric

Re: How should e2fsck clear s_errno/j_errno on an ro mount?

[Theodore Ts'o]

On Fri, Jul 20, 2012 at 06:13:50PM -0500, Eric Sandeen wrote:
> I'm looking at a situation where a root filesystem encountered an error and shut down, and therefore the error was stored in the journal.
> 
> But for the root fs, it seems that nothing can clear it.
> 
> If we do e2fsck -fy on a readonly mounted filesystem, then remount,rw the error persists:
> 
> [25124.319387] EXT4-fs warning (device loop3): ext4_clear_journal_err:4281: Filesystem error recorded from previous mount: IO failure
> [25124.331140] EXT4-fs warning (device loop3): ext4_clear_journal_err:4282: Marking fs in need of filesystem check.
> 
> ad infinitum.
> 
> It may be my fever-addled brain this week but I'm having a hard time following how this error is supposed to get set & cleared, especially if e2fsck has modified it while mounted ro.
> 
> As soon as I mount rw again, load_superblock() sees the journal superblock has an error set, and copies it back into the journal->j_errno.
> 
> After Ted's "e2fsck: correctly propagate error from journal to superblock" in e2fsprogs, at least an unmounted fs gets cleaned up, but I'm not sure what to do to fix this when it's mounted.  

The problem you describe should have been fixed by commit 6d75685e:
e2fsck: handle an already recovered journal with a non-zero s_error
field, which is in e2fsprogs 1.42.4.

					- Ted

Re: How should e2fsck clear s_errno/j_errno on an ro mount?

[Eric Sandeen]

On 7/20/12 7:39 PM, Theodore Ts'o wrote:
> On Fri, Jul 20, 2012 at 06:13:50PM -0500, Eric Sandeen wrote:
>> I'm looking at a situation where a root filesystem encountered an error and shut down, and therefore the error was stored in the journal.
>>
>> But for the root fs, it seems that nothing can clear it.
>>
>> If we do e2fsck -fy on a readonly mounted filesystem, then remount,rw the error persists:
>>
>> [25124.319387] EXT4-fs warning (device loop3): ext4_clear_journal_err:4281: Filesystem error recorded from previous mount: IO failure
>> [25124.331140] EXT4-fs warning (device loop3): ext4_clear_journal_err:4282: Marking fs in need of filesystem check.
>>
>> ad infinitum.
>>
>> It may be my fever-addled brain this week but I'm having a hard time following how this error is supposed to get set & cleared, especially if e2fsck has modified it while mounted ro.
>>
>> As soon as I mount rw again, load_superblock() sees the journal superblock has an error set, and copies it back into the journal->j_errno.
>>
>> After Ted's "e2fsck: correctly propagate error from journal to superblock" in e2fsprogs, at least an unmounted fs gets cleaned up, but I'm not sure what to do to fix this when it's mounted.  
> 
> The problem you describe should have been fixed by commit 6d75685e:
> e2fsck: handle an already recovered journal with a non-zero s_error
> field, which is in e2fsprogs 1.42.4.

Well, I tested e2fsprogs from git and saw the same trouble.  Let me look more closely at that commit.

-Eric

> 					- Ted
> 

Re: How should e2fsck clear s_errno/j_errno on an ro mount?

[Theodore Ts'o]

> nWell, I tested e2fsprogs from git and saw the same trouble.  Let me
> look more closely at that commit.

Hmm, would it be possible to send me /tmp/loop3.qcow.bz2 after running:

     e2image -Q /dev/loop3 /tmp/loop3.qcow
     bzip2 /tmp/loop3.qcow

Regardless of what the problem ends up being, this looks like
something for our regression test suite....

					- Ted

Re: How should e2fsck clear s_errno/j_errno on an ro mount?

[Eric Sandeen]

On 7/23/12 2:14 PM, Theodore Ts'o wrote:
>> nWell, I tested e2fsprogs from git and saw the same trouble.  Let me
>> look more closely at that commit.
> 
> Hmm, would it be possible to send me /tmp/loop3.qcow.bz2 after running:
> 
>      e2image -Q /dev/loop3 /tmp/loop3.qcow
>      bzip2 /tmp/loop3.qcow
> 
> Regardless of what the problem ends up being, this looks like
> something for our regression test suite....
> 
> 					- Ted
> 

Sure I can do that, will do it offline.

FWIW, the commit you mentioned changes e2fsck_check_ext3_journal, and
we only get there from main() like this:

        if ((ctx->mount_flags & EXT2_MF_MOUNTED) &&
            !(sb->s_feature_incompat & EXT3_FEATURE_INCOMPAT_RECOVER))
                goto skip_journal;

        retval = e2fsck_check_ext3_journal(ctx);

In my case I am mounted ro and recovery is done, so
EXT3_FEATURE_INCOMPAT_RECOVER is not, set, and so we skip over it with
the goto.

or else from here:

        if (sb->s_feature_incompat & EXT3_FEATURE_INCOMPAT_RECOVER) {
                if (ctx->options & E2F_OPT_READONLY) {
			...
                } else {
			...
                        retval = e2fsck_run_ext3_journal(ctx);

And again, I've already done recovery and am mounted RO so we won't go that way.

If make the first test above a little later:



diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 94260bd..73aa028 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -1424,10 +1424,6 @@ failure:
 		fprintf(ctx->logf, "Filesystem UUID: %s\n",
 			e2p_uuid2str(sb->s_uuid));
 
-	if ((ctx->mount_flags & EXT2_MF_MOUNTED) &&
-	    !(sb->s_feature_incompat & EXT3_FEATURE_INCOMPAT_RECOVER))
-		goto skip_journal;

Re: How should e2fsck clear s_errno/j_errno on an ro mount?

[Theodore Ts'o]

On Mon, Jul 23, 2012 at 03:21:45PM -0500, Eric Sandeen wrote:
> 
> If make the first test above a little later:
> 
> ...
> 
> 
> it clears it up for me but TBH I'm not totally clear on what all is going
> on here.

Your patch effectively reverts commit 47c1b8e1666: e2fsck: Skip journal
checks if the fs is mounted and doesn't need recovery

The reason why this patch was added was that it slightly speeded up
the HDD boots.  I've looked at this closely, and it's really not that
big of a speed up.  It only saves a single seek and read (to read the
journal superblock).  I've measured what it would be on a 1TB laptop
drive, and the difference is 60ms (70ms versus 10ms).

So I'm going to revert that commit, which should fix the problem that
we're seeing.  But it also turns out that the kernel is not quite
doing the right thing.  It should have never left the journal
superblock with a non-zero s_error indicator after it transfered the
error indicator to the superblock.  The problem was that we weren't
actually flushing the cleared errno field out to the disk for the
journal superblock.

							- Ted

[PATCH] ext4: make sure the journal sb is written in ext4_clear_journal_err()

[Theodore Ts'o]

After we transfer set the EXT4_ERROR_FS bit in the file system
superblock, it's not enough to call jbd2_journal_clear_err() to clear
the error indication from journal superblock --- we need to call
jbd2_journal_update_sb_errno() as well.  Otherwise, when the root file
system is mounted read-only, the journal is replayed, and the error
indicator is transferred to the superblock --- but the s_errno field
in the jbd2 superblock is left set (since although we cleared it in
memory, we never flushed it out to disk).

This can end up confusing e2fsck.  We should make e2fsck more robust
in this case, but the kernel shouldn't be leaving things in this
confused state, either.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: stable@kernel.org
---
 fs/ext4/super.c      | 1 +
 fs/jbd2/journal.c    | 3 ++-
 include/linux/jbd2.h | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a2a5979..ca09462 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4444,6 +4444,7 @@ static void ext4_clear_journal_err(struct super_block *sb,
 		ext4_commit_super(sb, 1);
 
 		jbd2_journal_clear_err(journal);
+		jbd2_journal_update_sb_errno(journal);
 	}
 }
 
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index e9a3c4c..bd23f2e 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1377,7 +1377,7 @@ static void jbd2_mark_journal_empty(journal_t *journal)
  * Update a journal's errno.  Write updated superblock to disk waiting for IO
  * to complete.
  */
-static void jbd2_journal_update_sb_errno(journal_t *journal)
+void jbd2_journal_update_sb_errno(journal_t *journal)
 {
 	journal_superblock_t *sb = journal->j_superblock;
 
@@ -1390,6 +1390,7 @@ static void jbd2_journal_update_sb_errno(journal_t *journal)
 
 	jbd2_write_superblock(journal, WRITE_SYNC);
 }
+EXPORT_SYMBOL(jbd2_journal_update_sb_errno);
 
 /*
  * Read the superblock for a given journal, performing initial
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index f334c7f..3efc43f 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1125,6 +1125,7 @@ extern int	   jbd2_journal_destroy    (journal_t *);
 extern int	   jbd2_journal_recover    (journal_t *journal);
 extern int	   jbd2_journal_wipe       (journal_t *, int);
 extern int	   jbd2_journal_skip_recovery	(journal_t *);
+extern void	   jbd2_journal_update_sb_errno(journal_t *);
 extern void	   jbd2_journal_update_sb_log_tail	(journal_t *, tid_t,
 				unsigned long, int);
 extern void	   __jbd2_journal_abort_hard	(journal_t *);
-- 
1.7.12.rc0.22.gcdd159b

[PATCH] Revert "e2fsck: Skip journal checks if the fs is mounted and doesn't need recovery"

[Theodore Ts'o]

This reverts commit 47c1b8e16668daa6e74cee3c7b8bdf237ffefe70.

The original reason for this commit was to speed up boots for hard
drives.  However, I've measured the time difference on a 1TB laptop
drive, and it's not significant: 70ms vs 10ms when running e2fsck on a
clean file system.

The problem with this optimization is that we don't notice if the
journal superblock has a non-zero s_errno field.  If we don't transfer
the error indicator from the journal superblock to the file system
superblock, then the kernel will transfer it when the file system is
remounted read-write, causing scary messages to appear in the syslog.
(And since there was a bug in the kernel code which didn't clear the
error indicator in the journal superblock, it would never get
cleared.)

Reported-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 e2fsck/unix.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 94260bd..f71a125 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -1424,10 +1424,6 @@ failure:
 		fprintf(ctx->logf, "Filesystem UUID: %s\n",
 			e2p_uuid2str(sb->s_uuid));
 
-	if ((ctx->mount_flags & EXT2_MF_MOUNTED) &&
-	    !(sb->s_feature_incompat & EXT3_FEATURE_INCOMPAT_RECOVER))
-		goto skip_journal;

[PATCH] e2fsck: check a file system mounted read-only if forced

[Theodore Ts'o]

Previously e2fsck would only allow a mounted file system to be checked
if it was the root file system and it was mounted read-only.  Now
allow any file system mounted read-only if the -f option is specified.

This makes it easier to test how e2fsck handles checking file systems
which are mounted without having to test on the root file system.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 e2fsck/unix.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index f71a125..da41f69 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -225,7 +225,9 @@ static void check_mount(e2fsck_t ctx)
 	     !(ctx->options & E2F_OPT_WRITECHECK)))
 		return;
 
-	if ((ctx->options & E2F_OPT_READONLY) &&
+	if (((ctx->options & E2F_OPT_READONLY) ||
+	     ((ctx->options & E2F_OPT_FORCE) &&
+	      (ctx->mount_flags & EXT2_MF_READONLY))) &&
 	    !(ctx->options & E2F_OPT_WRITECHECK)) {
 		log_out(ctx, _("Warning!  %s is %s.\n"),
 			ctx->filesystem_name,
@@ -1226,6 +1228,9 @@ restart:
 		if (!(ctx->mount_flags & EXT2_MF_ISROOT &&
 		      ctx->mount_flags & EXT2_MF_READONLY))
 			flags |= EXT2_FLAG_EXCLUSIVE;
+		if ((ctx->mount_flags & EXT2_MF_READONLY) &&
+		    (ctx->options & E2F_OPT_FORCE))
+			flags &= ~EXT2_FLAG_EXCLUSIVE;
 	}
 
 	retval = try_open_fs(ctx, flags, io_ptr, &fs);
-- 
1.7.12.rc0.22.gcdd159b