Re: [PATCH] ext4, jbd2: ensure entering into panic after recording an error in JBD2 superblock

Re: [PATCH] ext4, jbd2: ensure entering into panic after recording an error in JBD2 superblock

[Daeho Jeong]

Sorry, I will re-check that patch and I'll execute xfstest from now.

[PATCH] ext4, jbd2: ensure entering into panic after recording an error in JBD2 superblock

[Daeho Jeong]

If a EXT4 filesystem utilizes JBD2 journaling and an error occurs, the journaling
will be aborted first and the error number will be recorded into JBD2 superblock
and, finally, the system will enter into the panic state in "errors=panic" option.
But, in the rare case, this sequence is little twisted like the below figure and
it will happen that the system enters into panic state, which means the system
reset in mobile environment, before completion of recording an error in the
journal superblock. In this case, e2fsck cannot recognize that the filesystem
failure occured in the previous run and the corruption wouldn't be fixed.

Task A                                                 Task B
ext4_handle_error()
-> jbd2_journal_abort()
  -> __journal_abort_soft()
    -> __jbd2_journal_abort_hard()
    | -> journal->j_flags |= JBD2_ABORT;
    |                                                      __ext4_abort()
    |                                                      -> jbd2_journal_abort()
    |                                                      | -> __journal_abort_soft()
    |                                                      |   -> if (journal->j_flags & JBD2_ABORT)
    |                                                      |           return;
    |                                                      -> panic()
    -> jbd2_journal_update_sb_errno()

Tested-by: Hobin Woo <hobin.woo@samsung.com>
Signed-off-by: Daeho Jeong <daeho.jeong@samsung.com>
Signed-off-by: Youngjin Gil <youngjin.gil@samsung.com>
---
 fs/jbd2/journal.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index b96bd80..b265fd8 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2051,10 +2051,10 @@ static void __journal_abort_soft (journal_t *journal, int errno)
        if (!journal->j_errno)
                journal->j_errno = errno;

-       __jbd2_journal_abort_hard(journal);
-
        if (errno)
                jbd2_journal_update_sb_errno(journal);
+
+       __jbd2_journal_abort_hard(journal);
 }

 /**

Re: [PATCH] ext4, jbd2: ensure entering into panic after recording an error in JBD2 superblock

[Theodore Ts'o]

On Mon, Aug 10, 2015 at 05:44:49AM +0000, Daeho Jeong wrote:
> If a EXT4 filesystem utilizes JBD2 journaling and an error occurs, the journaling
> will be aborted first and the error number will be recorded into JBD2 superblock
> and, finally, the system will enter into the panic state in "errors=panic" option.
> But, in the rare case, this sequence is little twisted like the below figure and
> it will happen that the system enters into panic state, which means the system
> reset in mobile environment, before completion of recording an error in the
> journal superblock. In this case, e2fsck cannot recognize that the filesystem
> failure occured in the previous run and the corruption wouldn't be fixed.

Applied, but please note that the patch was very badly whitespace
damaged, *despite* being base64 encoded.  I had to manually apply the
patch.  I also adjusted the commit description so that its width was
no more 80 columns (ideally the width should be no more than 72
columns wide).

Thanks,

						- Ted

Re: [PATCH] ext4, jbd2: ensure entering into panic after recording an error in JBD2 superblock

[Theodore Ts'o]

On Sat, Aug 15, 2015 at 11:20:36AM -0400, Theodore Ts'o wrote:
> On Mon, Aug 10, 2015 at 05:44:49AM +0000, Daeho Jeong wrote:
> > If a EXT4 filesystem utilizes JBD2 journaling and an error occurs, the journaling
> > will be aborted first and the error number will be recorded into JBD2 superblock
> > and, finally, the system will enter into the panic state in "errors=panic" option.
> > But, in the rare case, this sequence is little twisted like the below figure and
> > it will happen that the system enters into panic state, which means the system
> > reset in mobile environment, before completion of recording an error in the
> > journal superblock. In this case, e2fsck cannot recognize that the filesystem
> > failure occured in the previous run and the corruption wouldn't be fixed.
> 
> Applied, but please note that the patch was very badly whitespace
> damaged, *despite* being base64 encoded.  I had to manually apply the
> patch.  I also adjusted the commit description so that its width was
> no more 80 columns (ideally the width should be no more than 72
> columns wide).

.... and this commit causes the kernel to wedge hard with xfstests
generic/081.  So I'm going to drop this patch.

As I believe I mentioned before, please consider using xfstests as
part of your development work flow; it will save both you and me time.

Thanks,

							- Ted