Hi,
This series originally aim to fix ext4_handle_error() and ext4_abort()
cannot panic because of we invoke __jbd2_journal_abort_hard() when we
failed to submit commit record without setting JBD2_REC_ERR flag.
I add patch 1 and patch 4 to switch to use jbd2_journal_abort() and do
some cleanup job at this iteration as Jan suggested. I also add patch 3
to fix missing updating ESHUTDOWN problem in commit 818d276ceb8 "ext4:
Add the journal checksum feature", please revirew this series and give
some suggestions.
Changes since v2:
- Fix spelling mistakes in the first patch.
- Keep JBD2_REC_ERR and remove the last place that invoke
jbd2_journal_abort() with 0 errno and the corresponding logic in
__journal_abort_soft().
- Fix missing updating errno in the jbd2 sb after jbd2 shutdown abort.
Thanks,
Yi.
zhangyi (F) (4):
jbd2: switch to use jbd2_journal_abort() when failed to submit the
commit record
ext4, jbd2: ensure panic when aborting with zero errno
jbd2: make sure ESHUTDOWN to be recorded in the journal superblock
jbd2: clean __jbd2_journal_abort_hard() and __journal_abort_soft()
fs/jbd2/checkpoint.c | 2 +-
fs/jbd2/commit.c | 4 +-
fs/jbd2/journal.c | 111 ++++++++++++++++---------------------------
include/linux/jbd2.h | 1 -
4 files changed, 45 insertions(+), 73 deletions(-)
--
2.17.2
__jbd2_journal_abort_hard() has never been used, now we can merge
__jbd2_journal_abort_hard() and __journal_abort_soft() these two
functions into jbd2_journal_abort() and remove them.
Signed-off-by: zhangyi (F) <[email protected]>
---
fs/jbd2/journal.c | 103 ++++++++++++++++++-------------------------
include/linux/jbd2.h | 1 -
2 files changed, 42 insertions(+), 62 deletions(-)
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 93be6e0311da..e59d9b6e4596 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -96,7 +96,6 @@ EXPORT_SYMBOL(jbd2_journal_release_jbd_inode);
EXPORT_SYMBOL(jbd2_journal_begin_ordered_truncate);
EXPORT_SYMBOL(jbd2_inode_cache);
-static void __journal_abort_soft (journal_t *journal, int errno);
static int jbd2_journal_create_slab(size_t slab_size);
#ifdef CONFIG_JBD2_DEBUG
@@ -805,7 +804,7 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,
"at offset %lu on %s\n",
__func__, blocknr, journal->j_devname);
err = -EIO;
- __journal_abort_soft(journal, err);
+ jbd2_journal_abort(journal, err);
}
} else {
*retp = blocknr; /* +journal->j_blk_offset */
@@ -2065,64 +2064,6 @@ int jbd2_journal_wipe(journal_t *journal, int write)
return err;
}
-/*
- * Journal abort has very specific semantics, which we describe
- * for journal abort.
- *
- * Two internal functions, which provide abort to the jbd layer
- * itself are here.
- */
-
-/*
- * Quick version for internal journal use (doesn't lock the journal).
- * Aborts hard --- we mark the abort as occurred, but do _nothing_ else,
- * and don't attempt to make any other journal updates.
- */
-void __jbd2_journal_abort_hard(journal_t *journal)
-{
- transaction_t *transaction;
-
- if (journal->j_flags & JBD2_ABORT)
- return;
-
- printk(KERN_ERR "Aborting journal on device %s.\n",
- journal->j_devname);
-
- write_lock(&journal->j_state_lock);
- journal->j_flags |= JBD2_ABORT;
- transaction = journal->j_running_transaction;
- if (transaction)
- __jbd2_log_start_commit(journal, transaction->t_tid);
- write_unlock(&journal->j_state_lock);
-}
-
-/* Soft abort: record the abort error status in the journal superblock,
- * but don't do any other IO. */
-static void __journal_abort_soft (journal_t *journal, int errno)
-{
- int old_errno;
-
- write_lock(&journal->j_state_lock);
- old_errno = journal->j_errno;
- if (!journal->j_errno || errno == -ESHUTDOWN)
- journal->j_errno = errno;
-
- if (journal->j_flags & JBD2_ABORT) {
- write_unlock(&journal->j_state_lock);
- if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN)
- jbd2_journal_update_sb_errno(journal);
- return;
- }
- write_unlock(&journal->j_state_lock);
-
- __jbd2_journal_abort_hard(journal);
-
- jbd2_journal_update_sb_errno(journal);
- write_lock(&journal->j_state_lock);
- journal->j_flags |= JBD2_REC_ERR;
- write_unlock(&journal->j_state_lock);
-}
-
/**
* void jbd2_journal_abort () - Shutdown the journal immediately.
* @journal: the journal to shutdown.
@@ -2166,7 +2107,47 @@ static void __journal_abort_soft (journal_t *journal, int errno)
void jbd2_journal_abort(journal_t *journal, int errno)
{
- __journal_abort_soft(journal, errno);
+ transaction_t *transaction;
+
+ /*
+ * ESHUTDOWN always takes precedence because a file system check
+ * caused by any other journal abort error is not required after
+ * a shutdown triggered.
+ */
+ write_lock(&journal->j_state_lock);
+ if (journal->j_flags & JBD2_ABORT) {
+ int old_errno = journal->j_errno;
+
+ write_unlock(&journal->j_state_lock);
+ if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN) {
+ journal->j_errno = errno;
+ jbd2_journal_update_sb_errno(journal);
+ }
+ return;
+ }
+
+ /*
+ * Mark the abort as occurred and start current running transaction
+ * to release all journaled buffer.
+ */
+ pr_err("Aborting journal on device %s.\n", journal->j_devname);
+
+ journal->j_flags |= JBD2_ABORT;
+ journal->j_errno = errno;
+ transaction = journal->j_running_transaction;
+ if (transaction)
+ __jbd2_log_start_commit(journal, transaction->t_tid);
+ write_unlock(&journal->j_state_lock);
+
+ /*
+ * Record errno to the journal super block, so that fsck and jbd2
+ * layer could realise that a filesystem check is needed.
+ */
+ jbd2_journal_update_sb_errno(journal);
+
+ write_lock(&journal->j_state_lock);
+ journal->j_flags |= JBD2_REC_ERR;
+ write_unlock(&journal->j_state_lock);
}
/**
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 603fbc4e2f70..e3e271bfb0e7 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1402,7 +1402,6 @@ extern int jbd2_journal_skip_recovery (journal_t *);
extern void jbd2_journal_update_sb_errno(journal_t *);
extern int jbd2_journal_update_sb_log_tail (journal_t *, tid_t,
unsigned long, int);
-extern void __jbd2_journal_abort_hard (journal_t *);
extern void jbd2_journal_abort (journal_t *, int);
extern int jbd2_journal_errno (journal_t *);
extern void jbd2_journal_ack_err (journal_t *);
--
2.17.2
Commit fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer") want
to allow jbd2 layer to distinguish shutdown journal abort from other
error cases. So the ESHUTDOWN should be taken precedence over any other
errno which has already been recoded after EXT4_FLAGS_SHUTDOWN is set,
but it only update errno in the journal suoerblock now if the old errno
is 0.
Fixes: fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer")
Signed-off-by: zhangyi (F) <[email protected]>
---
fs/jbd2/journal.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index b2d6e7666d0f..93be6e0311da 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2109,8 +2109,7 @@ static void __journal_abort_soft (journal_t *journal, int errno)
if (journal->j_flags & JBD2_ABORT) {
write_unlock(&journal->j_state_lock);
- if (!old_errno && old_errno != -ESHUTDOWN &&
- errno == -ESHUTDOWN)
+ if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN)
jbd2_journal_update_sb_errno(journal);
return;
}
--
2.17.2
JBD2_REC_ERR flag used to indicate the errno has been updated when jbd2
aborted, and then __ext4_abort() and ext4_handle_error() can invoke
panic if ERRORS_PANIC is specified. But if the journal has been aborted
with zero errno, jbd2_journal_abort() didn't set this flag so we can
no longer panic. Fix this by always record the proper errno in the
journal superblock.
Fixes: 4327ba52afd03 ("ext4, jbd2: ensure entering into panic after recording an error in superblock")
Signed-off-by: zhangyi (F) <[email protected]>
---
fs/jbd2/checkpoint.c | 2 +-
fs/jbd2/journal.c | 15 ++++-----------
2 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index a1909066bde6..62cf497f18eb 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -164,7 +164,7 @@ void __jbd2_log_wait_for_space(journal_t *journal)
"journal space in %s\n", __func__,
journal->j_devname);
WARN_ON(1);
- jbd2_journal_abort(journal, 0);
+ jbd2_journal_abort(journal, -EIO);
}
write_lock(&journal->j_state_lock);
} else {
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 1c58859aa592..b2d6e7666d0f 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2118,12 +2118,10 @@ static void __journal_abort_soft (journal_t *journal, int errno)
__jbd2_journal_abort_hard(journal);
- if (errno) {
- jbd2_journal_update_sb_errno(journal);
- write_lock(&journal->j_state_lock);
- journal->j_flags |= JBD2_REC_ERR;
- write_unlock(&journal->j_state_lock);
- }
+ jbd2_journal_update_sb_errno(journal);
+ write_lock(&journal->j_state_lock);
+ journal->j_flags |= JBD2_REC_ERR;
+ write_unlock(&journal->j_state_lock);
}
/**
@@ -2165,11 +2163,6 @@ static void __journal_abort_soft (journal_t *journal, int errno)
* failure to disk. ext3_error, for example, now uses this
* functionality.
*
- * Errors which originate from within the journaling layer will NOT
- * supply an errno; a null errno implies that absolutely no further
- * writes are done to the journal (unless there are any already in
- * progress).
- *
*/
void jbd2_journal_abort(journal_t *journal, int errno)
--
2.17.2
On Wed 04-12-19 20:46:12, zhangyi (F) wrote:
> JBD2_REC_ERR flag used to indicate the errno has been updated when jbd2
> aborted, and then __ext4_abort() and ext4_handle_error() can invoke
> panic if ERRORS_PANIC is specified. But if the journal has been aborted
> with zero errno, jbd2_journal_abort() didn't set this flag so we can
> no longer panic. Fix this by always record the proper errno in the
> journal superblock.
>
> Fixes: 4327ba52afd03 ("ext4, jbd2: ensure entering into panic after recording an error in superblock")
> Signed-off-by: zhangyi (F) <[email protected]>
Looks good to me. You can add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/jbd2/checkpoint.c | 2 +-
> fs/jbd2/journal.c | 15 ++++-----------
> 2 files changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index a1909066bde6..62cf497f18eb 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -164,7 +164,7 @@ void __jbd2_log_wait_for_space(journal_t *journal)
> "journal space in %s\n", __func__,
> journal->j_devname);
> WARN_ON(1);
> - jbd2_journal_abort(journal, 0);
> + jbd2_journal_abort(journal, -EIO);
> }
> write_lock(&journal->j_state_lock);
> } else {
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 1c58859aa592..b2d6e7666d0f 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -2118,12 +2118,10 @@ static void __journal_abort_soft (journal_t *journal, int errno)
>
> __jbd2_journal_abort_hard(journal);
>
> - if (errno) {
> - jbd2_journal_update_sb_errno(journal);
> - write_lock(&journal->j_state_lock);
> - journal->j_flags |= JBD2_REC_ERR;
> - write_unlock(&journal->j_state_lock);
> - }
> + jbd2_journal_update_sb_errno(journal);
> + write_lock(&journal->j_state_lock);
> + journal->j_flags |= JBD2_REC_ERR;
> + write_unlock(&journal->j_state_lock);
> }
>
> /**
> @@ -2165,11 +2163,6 @@ static void __journal_abort_soft (journal_t *journal, int errno)
> * failure to disk. ext3_error, for example, now uses this
> * functionality.
> *
> - * Errors which originate from within the journaling layer will NOT
> - * supply an errno; a null errno implies that absolutely no further
> - * writes are done to the journal (unless there are any already in
> - * progress).
> - *
> */
>
> void jbd2_journal_abort(journal_t *journal, int errno)
> --
> 2.17.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 04-12-19 20:46:13, zhangyi (F) wrote:
> Commit fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer") want
> to allow jbd2 layer to distinguish shutdown journal abort from other
> error cases. So the ESHUTDOWN should be taken precedence over any other
> errno which has already been recoded after EXT4_FLAGS_SHUTDOWN is set,
> but it only update errno in the journal suoerblock now if the old errno
> is 0.
>
> Fixes: fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer")
> Signed-off-by: zhangyi (F) <[email protected]>
Yeah, I think this is correct if I understand the logic correctly but I'd
like Ted to have a look at this. Anyway, feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/jbd2/journal.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index b2d6e7666d0f..93be6e0311da 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -2109,8 +2109,7 @@ static void __journal_abort_soft (journal_t *journal, int errno)
>
> if (journal->j_flags & JBD2_ABORT) {
> write_unlock(&journal->j_state_lock);
> - if (!old_errno && old_errno != -ESHUTDOWN &&
> - errno == -ESHUTDOWN)
> + if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN)
> jbd2_journal_update_sb_errno(journal);
> return;
> }
> --
> 2.17.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed 04-12-19 20:46:14, zhangyi (F) wrote:
> __jbd2_journal_abort_hard() has never been used, now we can merge
> __jbd2_journal_abort_hard() and __journal_abort_soft() these two
> functions into jbd2_journal_abort() and remove them.
>
> Signed-off-by: zhangyi (F) <[email protected]>
Looks good to me. You can add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/jbd2/journal.c | 103 ++++++++++++++++++-------------------------
> include/linux/jbd2.h | 1 -
> 2 files changed, 42 insertions(+), 62 deletions(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 93be6e0311da..e59d9b6e4596 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -96,7 +96,6 @@ EXPORT_SYMBOL(jbd2_journal_release_jbd_inode);
> EXPORT_SYMBOL(jbd2_journal_begin_ordered_truncate);
> EXPORT_SYMBOL(jbd2_inode_cache);
>
> -static void __journal_abort_soft (journal_t *journal, int errno);
> static int jbd2_journal_create_slab(size_t slab_size);
>
> #ifdef CONFIG_JBD2_DEBUG
> @@ -805,7 +804,7 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,
> "at offset %lu on %s\n",
> __func__, blocknr, journal->j_devname);
> err = -EIO;
> - __journal_abort_soft(journal, err);
> + jbd2_journal_abort(journal, err);
> }
> } else {
> *retp = blocknr; /* +journal->j_blk_offset */
> @@ -2065,64 +2064,6 @@ int jbd2_journal_wipe(journal_t *journal, int write)
> return err;
> }
>
> -/*
> - * Journal abort has very specific semantics, which we describe
> - * for journal abort.
> - *
> - * Two internal functions, which provide abort to the jbd layer
> - * itself are here.
> - */
> -
> -/*
> - * Quick version for internal journal use (doesn't lock the journal).
> - * Aborts hard --- we mark the abort as occurred, but do _nothing_ else,
> - * and don't attempt to make any other journal updates.
> - */
> -void __jbd2_journal_abort_hard(journal_t *journal)
> -{
> - transaction_t *transaction;
> -
> - if (journal->j_flags & JBD2_ABORT)
> - return;
> -
> - printk(KERN_ERR "Aborting journal on device %s.\n",
> - journal->j_devname);
> -
> - write_lock(&journal->j_state_lock);
> - journal->j_flags |= JBD2_ABORT;
> - transaction = journal->j_running_transaction;
> - if (transaction)
> - __jbd2_log_start_commit(journal, transaction->t_tid);
> - write_unlock(&journal->j_state_lock);
> -}
> -
> -/* Soft abort: record the abort error status in the journal superblock,
> - * but don't do any other IO. */
> -static void __journal_abort_soft (journal_t *journal, int errno)
> -{
> - int old_errno;
> -
> - write_lock(&journal->j_state_lock);
> - old_errno = journal->j_errno;
> - if (!journal->j_errno || errno == -ESHUTDOWN)
> - journal->j_errno = errno;
> -
> - if (journal->j_flags & JBD2_ABORT) {
> - write_unlock(&journal->j_state_lock);
> - if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN)
> - jbd2_journal_update_sb_errno(journal);
> - return;
> - }
> - write_unlock(&journal->j_state_lock);
> -
> - __jbd2_journal_abort_hard(journal);
> -
> - jbd2_journal_update_sb_errno(journal);
> - write_lock(&journal->j_state_lock);
> - journal->j_flags |= JBD2_REC_ERR;
> - write_unlock(&journal->j_state_lock);
> -}
> -
> /**
> * void jbd2_journal_abort () - Shutdown the journal immediately.
> * @journal: the journal to shutdown.
> @@ -2166,7 +2107,47 @@ static void __journal_abort_soft (journal_t *journal, int errno)
>
> void jbd2_journal_abort(journal_t *journal, int errno)
> {
> - __journal_abort_soft(journal, errno);
> + transaction_t *transaction;
> +
> + /*
> + * ESHUTDOWN always takes precedence because a file system check
> + * caused by any other journal abort error is not required after
> + * a shutdown triggered.
> + */
> + write_lock(&journal->j_state_lock);
> + if (journal->j_flags & JBD2_ABORT) {
> + int old_errno = journal->j_errno;
> +
> + write_unlock(&journal->j_state_lock);
> + if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN) {
> + journal->j_errno = errno;
> + jbd2_journal_update_sb_errno(journal);
> + }
> + return;
> + }
> +
> + /*
> + * Mark the abort as occurred and start current running transaction
> + * to release all journaled buffer.
> + */
> + pr_err("Aborting journal on device %s.\n", journal->j_devname);
> +
> + journal->j_flags |= JBD2_ABORT;
> + journal->j_errno = errno;
> + transaction = journal->j_running_transaction;
> + if (transaction)
> + __jbd2_log_start_commit(journal, transaction->t_tid);
> + write_unlock(&journal->j_state_lock);
> +
> + /*
> + * Record errno to the journal super block, so that fsck and jbd2
> + * layer could realise that a filesystem check is needed.
> + */
> + jbd2_journal_update_sb_errno(journal);
> +
> + write_lock(&journal->j_state_lock);
> + journal->j_flags |= JBD2_REC_ERR;
> + write_unlock(&journal->j_state_lock);
> }
>
> /**
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 603fbc4e2f70..e3e271bfb0e7 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1402,7 +1402,6 @@ extern int jbd2_journal_skip_recovery (journal_t *);
> extern void jbd2_journal_update_sb_errno(journal_t *);
> extern int jbd2_journal_update_sb_log_tail (journal_t *, tid_t,
> unsigned long, int);
> -extern void __jbd2_journal_abort_hard (journal_t *);
> extern void jbd2_journal_abort (journal_t *, int);
> extern int jbd2_journal_errno (journal_t *);
> extern void jbd2_journal_ack_err (journal_t *);
> --
> 2.17.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On 2019/12/5 1:05, Jan Kara wrote:
> On Wed 04-12-19 20:46:13, zhangyi (F) wrote:
>> Commit fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer") want
>> to allow jbd2 layer to distinguish shutdown journal abort from other
>> error cases. So the ESHUTDOWN should be taken precedence over any other
>> errno which has already been recoded after EXT4_FLAGS_SHUTDOWN is set,
>> but it only update errno in the journal suoerblock now if the old errno
>> is 0.
>>
>> Fixes: fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer")
>> Signed-off-by: zhangyi (F) <[email protected]>
>
> Yeah, I think this is correct if I understand the logic correctly but I'd
> like Ted to have a look at this. Anyway, feel free to add:
>
> Reviewed-by: Jan Kara <[email protected]>
>
Thanks for review.
Hi Ted, do you have time to look at this patch?
Thanks,
Yi.
>
>> ---
>> fs/jbd2/journal.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index b2d6e7666d0f..93be6e0311da 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -2109,8 +2109,7 @@ static void __journal_abort_soft (journal_t *journal, int errno)
>>
>> if (journal->j_flags & JBD2_ABORT) {
>> write_unlock(&journal->j_state_lock);
>> - if (!old_errno && old_errno != -ESHUTDOWN &&
>> - errno == -ESHUTDOWN)
>> + if (old_errno != -ESHUTDOWN && errno == -ESHUTDOWN)
>> jbd2_journal_update_sb_errno(journal);
>> return;
>> }
>> --
>> 2.17.2
>>
On 2019/12/5 9:23, zhangyi (F) wrote:
> On 2019/12/5 1:05, Jan Kara wrote:
>> On Wed 04-12-19 20:46:13, zhangyi (F) wrote:
>>> Commit fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer") want
>>> to allow jbd2 layer to distinguish shutdown journal abort from other
>>> error cases. So the ESHUTDOWN should be taken precedence over any other
>>> errno which has already been recoded after EXT4_FLAGS_SHUTDOWN is set,
>>> but it only update errno in the journal suoerblock now if the old errno
>>> is 0.
>>>
>>> Fixes: fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer")
>>> Signed-off-by: zhangyi (F) <[email protected]>
>>
>> Yeah, I think this is correct if I understand the logic correctly but I'd
>> like Ted to have a look at this. Anyway, feel free to add:
>>
>> Reviewed-by: Jan Kara <[email protected]>
>>
>
> Thanks for review.
>
> Hi Ted, do you have time to look at this patch?
>
Genteel ping.
Thanks,
Yi.
On Wed, Dec 04, 2019 at 01:52:13PM +0100, Jan Kara wrote:
> On Wed 04-12-19 20:46:12, zhangyi (F) wrote:
> > JBD2_REC_ERR flag used to indicate the errno has been updated when jbd2
> > aborted, and then __ext4_abort() and ext4_handle_error() can invoke
> > panic if ERRORS_PANIC is specified. But if the journal has been aborted
> > with zero errno, jbd2_journal_abort() didn't set this flag so we can
> > no longer panic. Fix this by always record the proper errno in the
> > journal superblock.
> >
> > Fixes: 4327ba52afd03 ("ext4, jbd2: ensure entering into panic after recording an error in superblock")
> > Signed-off-by: zhangyi (F) <[email protected]>
>
> Looks good to me. You can add:
>
> Reviewed-by: Jan Kara <[email protected]>
Applied, thanks.
- Ted
On Wed, Dec 04, 2019 at 06:05:28PM +0100, Jan Kara wrote:
> On Wed 04-12-19 20:46:13, zhangyi (F) wrote:
> > Commit fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer") want
> > to allow jbd2 layer to distinguish shutdown journal abort from other
> > error cases. So the ESHUTDOWN should be taken precedence over any other
> > errno which has already been recoded after EXT4_FLAGS_SHUTDOWN is set,
> > but it only update errno in the journal suoerblock now if the old errno
> > is 0.
> >
> > Fixes: fb7c02445c49 ("ext4: pass -ESHUTDOWN code to jbd2 layer")
> > Signed-off-by: zhangyi (F) <[email protected]>
>
> Yeah, I think this is correct if I understand the logic correctly but I'd
> like Ted to have a look at this. Anyway, feel free to add:
>
> Reviewed-by: Jan Kara <[email protected]>
Yep, this looks sane. Thanks, applied.
- Ted
On Wed, Dec 04, 2019 at 08:46:14PM +0800, zhangyi (F) wrote:
> __jbd2_journal_abort_hard() has never been used, now we can merge
> __jbd2_journal_abort_hard() and __journal_abort_soft() these two
> functions into jbd2_journal_abort() and remove them.
>
> Signed-off-by: zhangyi (F) <[email protected]>
Thanks, applied.
- Ted