* [PATCH v4 0/2] fix error flag covered by journal recovery
@ 2023-03-01 11:59 Ye Bin
2023-03-01 11:59 ` [PATCH v4 1/2] ext4: commit super block if fs record error when journal record without error Ye Bin
2023-03-01 11:59 ` [PATCH v4 2/2] ext4: make sure fs error flag setted before clear journal error Ye Bin
0 siblings, 2 replies; 6+ messages in thread
From: Ye Bin @ 2023-03-01 11:59 UTC (permalink / raw
To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin
From: Ye Bin <yebin10@huawei.com>
Diff v4 Vs v3:
After journal replay recover 'es->s_state' error flag like recover error
info.
Diff v3 Vs v2:
Only fix fs error flag lost when previous journal errno is not record
in disk. As this may lead to drop orphan list, however fs not record
error flag, then fsck will not repair deeply.
Diff v2 vs v1:
Move call 'j_replay_prepare_callback' and 'j_replay_end_callback' from
ext4_load_journal() to jbd2_journal_recover().
When do fault injection test, got issue as follows:
EXT4-fs (dm-5): warning: mounting fs with errors, running e2fsck is recommended
EXT4-fs (dm-5): Errors on filesystem, clearing orphan list.
EXT4-fs (dm-5): recovery complete
EXT4-fs (dm-5): mounted filesystem with ordered data mode. Opts: data_err=abort,errors=remount-ro
EXT4-fs (dm-5): recovery complete
EXT4-fs (dm-5): mounted filesystem with ordered data mode. Opts: data_err=abort,errors=remount-ro
Without do file system check, file system is clean when do second mount.
Theoretically, the kernel will not clear fs error flag. In errors=remount-ro
mode the last super block is commit directly. So super block in journal is
not uptodate. When do jounral recovery, the uptodate super block will be
covered by jounral data. If super block submit all failed after recover
journal, then file system error flag is lost. When do "fsck -a" couldn't
repair file system deeply.
To solve above issue we need to do extra handle when do super block journal
recovery.
Ye Bin (2):
ext4: commit super block if fs record error when journal record
without error
ext4: make sure fs error flag setted before clear journal error
fs/ext4/super.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/2] ext4: commit super block if fs record error when journal record without error
2023-03-01 11:59 [PATCH v4 0/2] fix error flag covered by journal recovery Ye Bin
@ 2023-03-01 11:59 ` Ye Bin
2023-03-01 13:03 ` Jan Kara
2023-03-02 1:30 ` Baokun Li
2023-03-01 11:59 ` [PATCH v4 2/2] ext4: make sure fs error flag setted before clear journal error Ye Bin
1 sibling, 2 replies; 6+ messages in thread
From: Ye Bin @ 2023-03-01 11:59 UTC (permalink / raw
To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin
From: Ye Bin <yebin10@huawei.com>
Now, 'es->s_state' maybe covered by recover journal. And journal errno
maybe not recorded in journal sb as IO error. ext4_update_super() only
update error information when 'sbi->s_add_error_count' large than zero.
Then 'EXT4_ERROR_FS' flag maybe lost.
To solve above issue just recover 'es->s_state' error flag after journal
replay like error info.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
fs/ext4/super.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index faae05493471..9df8fada2dce 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5910,7 +5910,9 @@ static int ext4_load_journal(struct super_block *sb,
if (!ext4_has_feature_journal_needs_recovery(sb))
err = jbd2_journal_wipe(journal, !really_read_only);
if (!err) {
+ int err2;
char *save = kmalloc(EXT4_S_ERR_LEN, GFP_KERNEL);
+
if (save)
memcpy(save, ((char *) es) +
EXT4_S_ERR_START, EXT4_S_ERR_LEN);
@@ -5919,6 +5921,11 @@ static int ext4_load_journal(struct super_block *sb,
memcpy(((char *) es) + EXT4_S_ERR_START,
save, EXT4_S_ERR_LEN);
kfree(save);
+ es->s_state |= cpu_to_le16(EXT4_SB(sb)->s_mount_state &
+ EXT4_ERROR_FS);
+ /* Write out restored error information to the superblock */
+ err2 = ext4_commit_super(sb);
+ err = err ? : err2;
}
if (err) {
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 2/2] ext4: make sure fs error flag setted before clear journal error
2023-03-01 11:59 [PATCH v4 0/2] fix error flag covered by journal recovery Ye Bin
2023-03-01 11:59 ` [PATCH v4 1/2] ext4: commit super block if fs record error when journal record without error Ye Bin
@ 2023-03-01 11:59 ` Ye Bin
2023-03-02 1:26 ` Baokun Li
1 sibling, 1 reply; 6+ messages in thread
From: Ye Bin @ 2023-03-01 11:59 UTC (permalink / raw
To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin
From: Ye Bin <yebin10@huawei.com>
Now, jounral error number maybe cleared even though ext4_commit_super()
failed. This may lead to error flag miss, then fsck will miss to check
file system deeply.
Signed-off-by: Ye Bin <yebin10@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
fs/ext4/super.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 9df8fada2dce..7736c96d5a78 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6152,11 +6152,13 @@ static int ext4_clear_journal_err(struct super_block *sb,
errstr = ext4_decode_error(sb, j_errno, nbuf);
ext4_warning(sb, "Filesystem error recorded "
"from previous mount: %s", errstr);
- ext4_warning(sb, "Marking fs in need of filesystem check.");
EXT4_SB(sb)->s_mount_state |= EXT4_ERROR_FS;
es->s_state |= cpu_to_le16(EXT4_ERROR_FS);
- ext4_commit_super(sb);
+ j_errno = ext4_commit_super(sb);
+ if (j_errno)
+ return j_errno;
+ ext4_warning(sb, "Marked fs in need of filesystem check.");
jbd2_journal_clear_err(journal);
jbd2_journal_update_sb_errno(journal);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] ext4: commit super block if fs record error when journal record without error
2023-03-01 11:59 ` [PATCH v4 1/2] ext4: commit super block if fs record error when journal record without error Ye Bin
@ 2023-03-01 13:03 ` Jan Kara
2023-03-02 1:30 ` Baokun Li
1 sibling, 0 replies; 6+ messages in thread
From: Jan Kara @ 2023-03-01 13:03 UTC (permalink / raw
To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, jack, Ye Bin
On Wed 01-03-23 19:59:08, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> Now, 'es->s_state' maybe covered by recover journal. And journal errno
> maybe not recorded in journal sb as IO error. ext4_update_super() only
> update error information when 'sbi->s_add_error_count' large than zero.
> Then 'EXT4_ERROR_FS' flag maybe lost.
> To solve above issue just recover 'es->s_state' error flag after journal
> replay like error info.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
Thanks. The patch looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/super.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index faae05493471..9df8fada2dce 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5910,7 +5910,9 @@ static int ext4_load_journal(struct super_block *sb,
> if (!ext4_has_feature_journal_needs_recovery(sb))
> err = jbd2_journal_wipe(journal, !really_read_only);
> if (!err) {
> + int err2;
> char *save = kmalloc(EXT4_S_ERR_LEN, GFP_KERNEL);
> +
> if (save)
> memcpy(save, ((char *) es) +
> EXT4_S_ERR_START, EXT4_S_ERR_LEN);
> @@ -5919,6 +5921,11 @@ static int ext4_load_journal(struct super_block *sb,
> memcpy(((char *) es) + EXT4_S_ERR_START,
> save, EXT4_S_ERR_LEN);
> kfree(save);
> + es->s_state |= cpu_to_le16(EXT4_SB(sb)->s_mount_state &
> + EXT4_ERROR_FS);
> + /* Write out restored error information to the superblock */
> + err2 = ext4_commit_super(sb);
> + err = err ? : err2;
> }
>
> if (err) {
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] ext4: make sure fs error flag setted before clear journal error
2023-03-01 11:59 ` [PATCH v4 2/2] ext4: make sure fs error flag setted before clear journal error Ye Bin
@ 2023-03-02 1:26 ` Baokun Li
0 siblings, 0 replies; 6+ messages in thread
From: Baokun Li @ 2023-03-02 1:26 UTC (permalink / raw
To: Ye Bin, tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin
On 2023/3/1 19:59, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> Now, jounral error number maybe cleared even though ext4_commit_super()
> failed. This may lead to error flag miss, then fsck will miss to check
> file system deeply.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
Looks good to me.
Reviewed-by: Baokun Li <libaokun1@huawei.com>
> ---
> fs/ext4/super.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 9df8fada2dce..7736c96d5a78 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6152,11 +6152,13 @@ static int ext4_clear_journal_err(struct super_block *sb,
> errstr = ext4_decode_error(sb, j_errno, nbuf);
> ext4_warning(sb, "Filesystem error recorded "
> "from previous mount: %s", errstr);
> - ext4_warning(sb, "Marking fs in need of filesystem check.");
>
> EXT4_SB(sb)->s_mount_state |= EXT4_ERROR_FS;
> es->s_state |= cpu_to_le16(EXT4_ERROR_FS);
> - ext4_commit_super(sb);
> + j_errno = ext4_commit_super(sb);
> + if (j_errno)
> + return j_errno;
> + ext4_warning(sb, "Marked fs in need of filesystem check.");
>
> jbd2_journal_clear_err(journal);
> jbd2_journal_update_sb_errno(journal);
--
With Best Regards,
Baokun Li
.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] ext4: commit super block if fs record error when journal record without error
2023-03-01 11:59 ` [PATCH v4 1/2] ext4: commit super block if fs record error when journal record without error Ye Bin
2023-03-01 13:03 ` Jan Kara
@ 2023-03-02 1:30 ` Baokun Li
1 sibling, 0 replies; 6+ messages in thread
From: Baokun Li @ 2023-03-02 1:30 UTC (permalink / raw
To: Ye Bin, tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin
On 2023/3/1 19:59, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> Now, 'es->s_state' maybe covered by recover journal. And journal errno
> maybe not recorded in journal sb as IO error. ext4_update_super() only
> update error information when 'sbi->s_add_error_count' large than zero.
> Then 'EXT4_ERROR_FS' flag maybe lost.
> To solve above issue just recover 'es->s_state' error flag after journal
> replay like error info.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
Looks good to me.
Reviewed-by: Baokun Li <libaokun1@huawei.com>
> ---
> fs/ext4/super.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index faae05493471..9df8fada2dce 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5910,7 +5910,9 @@ static int ext4_load_journal(struct super_block *sb,
> if (!ext4_has_feature_journal_needs_recovery(sb))
> err = jbd2_journal_wipe(journal, !really_read_only);
> if (!err) {
> + int err2;
> char *save = kmalloc(EXT4_S_ERR_LEN, GFP_KERNEL);
> +
> if (save)
> memcpy(save, ((char *) es) +
> EXT4_S_ERR_START, EXT4_S_ERR_LEN);
> @@ -5919,6 +5921,11 @@ static int ext4_load_journal(struct super_block *sb,
> memcpy(((char *) es) + EXT4_S_ERR_START,
> save, EXT4_S_ERR_LEN);
> kfree(save);
> + es->s_state |= cpu_to_le16(EXT4_SB(sb)->s_mount_state &
> + EXT4_ERROR_FS);
> + /* Write out restored error information to the superblock */
> + err2 = ext4_commit_super(sb);
> + err = err ? : err2;
> }
>
> if (err) {
--
With Best Regards,
Baokun Li
.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-02 1:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-01 11:59 [PATCH v4 0/2] fix error flag covered by journal recovery Ye Bin
2023-03-01 11:59 ` [PATCH v4 1/2] ext4: commit super block if fs record error when journal record without error Ye Bin
2023-03-01 13:03 ` Jan Kara
2023-03-02 1:30 ` Baokun Li
2023-03-01 11:59 ` [PATCH v4 2/2] ext4: make sure fs error flag setted before clear journal error Ye Bin
2023-03-02 1:26 ` Baokun Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.