2023-03-15 01:32:58

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v3 0/6] ext4: Fix stale buffer loading from last failed

Patch 1 fixes reusing stale buffer heads from last failed mounting.
Patch 2~4 reconstructs 'j_format_version' initialization and checking
in loading process.

v1->v2:
Adopt suggestions from Tudor, add fix tag and corrupt 'stable' field
in patch 1.
Reserve empty lines in patch 4.
v2->v3:
Split block device checking cleanup into a new patch (2th).
Add 'Reviewed-by' tag in patch 3-6.


Zhang Yi (4):
jbd2: remove unused feature macros
jbd2: switch to check format version in superblock directly
jbd2: factor out journal initialization from journal_get_superblock()
jbd2: remove j_format_version

Zhihao Cheng (2):
ext4: Fix reusing stale buffer heads from last failed mounting
ext4: ext4_put_super: Remove redundant checking for
'sbi->s_journal_bdev'

fs/ext4/super.c | 15 +++++++------
fs/jbd2/journal.c | 53 +++++++++++++++++---------------------------
include/linux/jbd2.h | 33 ++++++++++++---------------
3 files changed, 42 insertions(+), 59 deletions(-)

--
2.31.1



2023-03-15 01:33:02

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v3 1/6] ext4: Fix reusing stale buffer heads from last failed mounting

Following process makes ext4 load stale buffer heads from last failed
mounting in a new mounting operation:
mount_bdev
ext4_fill_super
| ext4_load_and_init_journal
| ext4_load_journal
| jbd2_journal_load
| load_superblock
| journal_get_superblock
| set_buffer_verified(bh) // buffer head is verified
| jbd2_journal_recover // failed caused by EIO
| goto failed_mount3a // skip 'sb->s_root' initialization
deactivate_locked_super
kill_block_super
generic_shutdown_super
if (sb->s_root)
// false, skip ext4_put_super->invalidate_bdev->
// invalidate_mapping_pages->mapping_evict_folio->
// filemap_release_folio->try_to_free_buffers, which
// cannot drop buffer head.
blkdev_put
blkdev_put_whole
if (atomic_dec_and_test(&bdev->bd_openers))
// false, systemd-udev happens to open the device. Then
// blkdev_flush_mapping->kill_bdev->truncate_inode_pages->
// truncate_inode_folio->truncate_cleanup_folio->
// folio_invalidate->block_invalidate_folio->
// filemap_release_folio->try_to_free_buffers will be skipped,
// dropping buffer head is missed again.

Second mount:
ext4_fill_super
ext4_load_and_init_journal
ext4_load_journal
ext4_get_journal
jbd2_journal_init_inode
journal_init_common
bh = getblk_unmovable
bh = __find_get_block // Found stale bh in last failed mounting
journal->j_sb_buffer = bh
jbd2_journal_load
load_superblock
journal_get_superblock
if (buffer_verified(bh))
// true, skip journal->j_format_version = 2, value is 0
jbd2_journal_recover
do_one_pass
next_log_block += count_tags(journal, bh)
// According to journal_tag_bytes(), 'tag_bytes' calculating is
// affected by jbd2_has_feature_csum3(), jbd2_has_feature_csum3()
// returns false because 'j->j_format_version >= 2' is not true,
// then we get wrong next_log_block. The do_one_pass may exit
// early whenoccuring non JBD2_MAGIC_NUMBER in 'next_log_block'.

The filesystem is corrupted here, journal is partially replayed, and
new journal sequence number actually is already used by last mounting.

The invalidate_bdev() can drop all buffer heads even racing with bare
reading block device(eg. systemd-udev), so we can fix it by invalidating
bdev in error handling path in __ext4_fill_super().

Fetch a reproducer in [Link].

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217171
Fixes: 25ed6e8a54df ("jbd2: enable journal clients to enable v2 checksumming")
Cc: [email protected] # v3.5
Signed-off-by: Zhihao Cheng <[email protected]>
---
fs/ext4/super.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index f43e526112ae..61511b7ba017 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1126,6 +1126,12 @@ static void ext4_blkdev_remove(struct ext4_sb_info *sbi)
struct block_device *bdev;
bdev = sbi->s_journal_bdev;
if (bdev) {
+ /*
+ * Invalidate the journal device's buffers. We don't want them
+ * floating about in memory - the physical journal device may
+ * hotswapped, and it breaks the `ro-after' testing code.
+ */
+ invalidate_bdev(bdev);
ext4_blkdev_put(bdev);
sbi->s_journal_bdev = NULL;
}
@@ -1272,13 +1278,7 @@ static void ext4_put_super(struct super_block *sb)
sync_blockdev(sb->s_bdev);
invalidate_bdev(sb->s_bdev);
if (sbi->s_journal_bdev && sbi->s_journal_bdev != sb->s_bdev) {
- /*
- * Invalidate the journal device's buffers. We don't want them
- * floating about in memory - the physical journal device may
- * hotswapped, and it breaks the `ro-after' testing code.
- */
sync_blockdev(sbi->s_journal_bdev);
- invalidate_bdev(sbi->s_journal_bdev);
ext4_blkdev_remove(sbi);
}

@@ -5610,6 +5610,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
brelse(sbi->s_sbh);
ext4_blkdev_remove(sbi);
out_fail:
+ invalidate_bdev(sb->s_bdev);
sb->s_fs_info = NULL;
return err ? err : ret;
}
--
2.31.1


2023-03-15 01:33:05

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v3 2/6] ext4: ext4_put_super: Remove redundant checking for 'sbi->s_journal_bdev'

As discussed in [1], 'sbi->s_journal_bdev != sb->s_bdev' will always
become true if sbi->s_journal_bdev exists. Filesystem block device and
journal block device are both opened with 'FMODE_EXCL' mode, so these
two devices can't be same one. Then we can remove the redundant checking
'sbi->s_journal_bdev != sb->s_bdev' if 'sbi->s_journal_bdev' exists.

[1] https://lore.kernel.org/lkml/[email protected]/

Signed-off-by: Zhihao Cheng <[email protected]>
---
fs/ext4/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 61511b7ba017..a22417d113ca 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1277,7 +1277,7 @@ static void ext4_put_super(struct super_block *sb)

sync_blockdev(sb->s_bdev);
invalidate_bdev(sb->s_bdev);
- if (sbi->s_journal_bdev && sbi->s_journal_bdev != sb->s_bdev) {
+ if (sbi->s_journal_bdev) {
sync_blockdev(sbi->s_journal_bdev);
ext4_blkdev_remove(sbi);
}
--
2.31.1


2023-03-15 01:33:09

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v3 3/6] jbd2: remove unused feature macros

From: Zhang Yi <[email protected]>

JBD2_HAS_[IN|RO_]COMPAT_FEATURE macros are no longer used, just remove
them.

Signed-off-by: Zhang Yi <[email protected]>
Signed-off-by: Zhihao Cheng <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
include/linux/jbd2.h | 11 -----------
1 file changed, 11 deletions(-)

diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index f619bae1dcc5..a91cf9c7a94b 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -274,17 +274,6 @@ typedef struct journal_superblock_s
/* 0x0400 */
} journal_superblock_t;

-/* Use the jbd2_{has,set,clear}_feature_* helpers; these will be removed */
-#define JBD2_HAS_COMPAT_FEATURE(j,mask) \
- ((j)->j_format_version >= 2 && \
- ((j)->j_superblock->s_feature_compat & cpu_to_be32((mask))))
-#define JBD2_HAS_RO_COMPAT_FEATURE(j,mask) \
- ((j)->j_format_version >= 2 && \
- ((j)->j_superblock->s_feature_ro_compat & cpu_to_be32((mask))))
-#define JBD2_HAS_INCOMPAT_FEATURE(j,mask) \
- ((j)->j_format_version >= 2 && \
- ((j)->j_superblock->s_feature_incompat & cpu_to_be32((mask))))
-
#define JBD2_FEATURE_COMPAT_CHECKSUM 0x00000001

#define JBD2_FEATURE_INCOMPAT_REVOKE 0x00000001
--
2.31.1


2023-03-15 01:33:14

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v3 4/6] jbd2: switch to check format version in superblock directly

From: Zhang Yi <[email protected]>

We should only check and set extented features if journal format version
is 2, and now we check the in memory copy of the superblock
'journal->j_format_version', which relys on the parameter initialization
sequence, switch to use the h_blocktype in superblock cloud be more
clear.

Signed-off-by: Zhang Yi <[email protected]>
Signed-off-by: Zhihao Cheng <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/jbd2/journal.c | 16 +++++++---------
include/linux/jbd2.h | 17 ++++++++++++++---
2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 8ae419152ff6..8d5fe6738cc4 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2062,10 +2062,12 @@ int jbd2_journal_load(journal_t *journal)
return err;

sb = journal->j_superblock;
- /* If this is a V2 superblock, then we have to check the
- * features flags on it. */

- if (journal->j_format_version >= 2) {
+ /*
+ * If this is a V2 superblock, then we have to check the
+ * features flags on it.
+ */
+ if (jbd2_format_support_feature(journal)) {
if ((sb->s_feature_ro_compat &
~cpu_to_be32(JBD2_KNOWN_ROCOMPAT_FEATURES)) ||
(sb->s_feature_incompat &
@@ -2227,7 +2229,7 @@ int jbd2_journal_check_used_features(journal_t *journal, unsigned long compat,
if (journal->j_format_version == 0 &&
journal_get_superblock(journal) != 0)
return 0;
- if (journal->j_format_version == 1)
+ if (!jbd2_format_support_feature(journal))
return 0;

sb = journal->j_superblock;
@@ -2257,11 +2259,7 @@ int jbd2_journal_check_available_features(journal_t *journal, unsigned long comp
if (!compat && !ro && !incompat)
return 1;

- /* We can support any known requested features iff the
- * superblock is in version 2. Otherwise we fail to support any
- * extended sb features. */
-
- if (journal->j_format_version != 2)
+ if (!jbd2_format_support_feature(journal))
return 0;

if ((compat & JBD2_KNOWN_COMPAT_FEATURES) == compat &&
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index a91cf9c7a94b..1ffcea5c024e 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1313,11 +1313,22 @@ struct journal_s
rwsem_release(&j->j_trans_commit_map, _THIS_IP_); \
} while (0)

+/*
+ * We can support any known requested features iff the
+ * superblock is not in version 1. Otherwise we fail to support any
+ * extended sb features.
+ */
+static inline bool jbd2_format_support_feature(journal_t *j)
+{
+ return j->j_superblock->s_header.h_blocktype !=
+ cpu_to_be32(JBD2_SUPERBLOCK_V1);
+}
+
/* journal feature predicate functions */
#define JBD2_FEATURE_COMPAT_FUNCS(name, flagname) \
static inline bool jbd2_has_feature_##name(journal_t *j) \
{ \
- return ((j)->j_format_version >= 2 && \
+ return (jbd2_format_support_feature(j) && \
((j)->j_superblock->s_feature_compat & \
cpu_to_be32(JBD2_FEATURE_COMPAT_##flagname)) != 0); \
} \
@@ -1335,7 +1346,7 @@ static inline void jbd2_clear_feature_##name(journal_t *j) \
#define JBD2_FEATURE_RO_COMPAT_FUNCS(name, flagname) \
static inline bool jbd2_has_feature_##name(journal_t *j) \
{ \
- return ((j)->j_format_version >= 2 && \
+ return (jbd2_format_support_feature(j) && \
((j)->j_superblock->s_feature_ro_compat & \
cpu_to_be32(JBD2_FEATURE_RO_COMPAT_##flagname)) != 0); \
} \
@@ -1353,7 +1364,7 @@ static inline void jbd2_clear_feature_##name(journal_t *j) \
#define JBD2_FEATURE_INCOMPAT_FUNCS(name, flagname) \
static inline bool jbd2_has_feature_##name(journal_t *j) \
{ \
- return ((j)->j_format_version >= 2 && \
+ return (jbd2_format_support_feature(j) && \
((j)->j_superblock->s_feature_incompat & \
cpu_to_be32(JBD2_FEATURE_INCOMPAT_##flagname)) != 0); \
} \
--
2.31.1


2023-03-15 01:33:17

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v3 5/6] jbd2: factor out journal initialization from journal_get_superblock()

From: Zhang Yi <[email protected]>

Current journal_get_superblock() couple journal superblock checking and
partial journal initialization, factor out initialization part from it
to make things clear.

Signed-off-by: Zhang Yi <[email protected]>
Signed-off-by: Zhihao Cheng <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/jbd2/journal.c | 46 ++++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 8d5fe6738cc4..ee678f9e40c4 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1925,21 +1925,13 @@ static int journal_get_superblock(journal_t *journal)
goto out;
}

- switch(be32_to_cpu(sb->s_header.h_blocktype)) {
- case JBD2_SUPERBLOCK_V1:
- journal->j_format_version = 1;
- break;
- case JBD2_SUPERBLOCK_V2:
- journal->j_format_version = 2;
- break;
- default:
+ if (be32_to_cpu(sb->s_header.h_blocktype) != JBD2_SUPERBLOCK_V1 &&
+ be32_to_cpu(sb->s_header.h_blocktype) != JBD2_SUPERBLOCK_V2) {
printk(KERN_WARNING "JBD2: unrecognised superblock format ID\n");
goto out;
}

- if (be32_to_cpu(sb->s_maxlen) < journal->j_total_len)
- journal->j_total_len = be32_to_cpu(sb->s_maxlen);
- else if (be32_to_cpu(sb->s_maxlen) > journal->j_total_len) {
+ if (be32_to_cpu(sb->s_maxlen) > journal->j_total_len) {
printk(KERN_WARNING "JBD2: journal file too short\n");
goto out;
}
@@ -1982,25 +1974,14 @@ static int journal_get_superblock(journal_t *journal)
journal->j_chksum_driver = NULL;
goto out;
}
- }
-
- if (jbd2_journal_has_csum_v2or3(journal)) {
/* Check superblock checksum */
if (sb->s_checksum != jbd2_superblock_csum(journal, sb)) {
printk(KERN_ERR "JBD2: journal checksum error\n");
err = -EFSBADCRC;
goto out;
}
-
- /* Precompute checksum seed for all metadata */
- journal->j_csum_seed = jbd2_chksum(journal, ~0, sb->s_uuid,
- sizeof(sb->s_uuid));
}
-
- journal->j_revoke_records_per_block =
- journal_revoke_records_per_block(journal);
set_buffer_verified(bh);
-
return 0;

out:
@@ -2025,12 +2006,30 @@ static int load_superblock(journal_t *journal)

sb = journal->j_superblock;

+ switch (be32_to_cpu(sb->s_header.h_blocktype)) {
+ case JBD2_SUPERBLOCK_V1:
+ journal->j_format_version = 1;
+ break;
+ case JBD2_SUPERBLOCK_V2:
+ journal->j_format_version = 2;
+ break;
+ }
+
journal->j_tail_sequence = be32_to_cpu(sb->s_sequence);
journal->j_tail = be32_to_cpu(sb->s_start);
journal->j_first = be32_to_cpu(sb->s_first);
journal->j_errno = be32_to_cpu(sb->s_errno);
journal->j_last = be32_to_cpu(sb->s_maxlen);

+ if (be32_to_cpu(sb->s_maxlen) < journal->j_total_len)
+ journal->j_total_len = be32_to_cpu(sb->s_maxlen);
+ /* Precompute checksum seed for all metadata */
+ if (jbd2_journal_has_csum_v2or3(journal))
+ journal->j_csum_seed = jbd2_chksum(journal, ~0, sb->s_uuid,
+ sizeof(sb->s_uuid));
+ journal->j_revoke_records_per_block =
+ journal_revoke_records_per_block(journal);
+
if (jbd2_has_feature_fast_commit(journal)) {
journal->j_fc_last = be32_to_cpu(sb->s_maxlen);
num_fc_blocks = jbd2_journal_get_num_fc_blks(sb);
@@ -2226,8 +2225,7 @@ int jbd2_journal_check_used_features(journal_t *journal, unsigned long compat,
if (!compat && !ro && !incompat)
return 1;
/* Load journal superblock if it is not loaded yet. */
- if (journal->j_format_version == 0 &&
- journal_get_superblock(journal) != 0)
+ if (journal_get_superblock(journal))
return 0;
if (!jbd2_format_support_feature(journal))
return 0;
--
2.31.1


2023-03-15 01:33:21

by Zhihao Cheng

[permalink] [raw]
Subject: [PATCH v3 6/6] jbd2: remove j_format_version

From: Zhang Yi <[email protected]>

journal->j_format_version is no longer used, remove it.

Signed-off-by: Zhang Yi <[email protected]>
Signed-off-by: Zhihao Cheng <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/jbd2/journal.c | 9 ---------
include/linux/jbd2.h | 5 -----
2 files changed, 14 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index ee678f9e40c4..837a9a85e585 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2006,15 +2006,6 @@ static int load_superblock(journal_t *journal)

sb = journal->j_superblock;

- switch (be32_to_cpu(sb->s_header.h_blocktype)) {
- case JBD2_SUPERBLOCK_V1:
- journal->j_format_version = 1;
- break;
- case JBD2_SUPERBLOCK_V2:
- journal->j_format_version = 2;
- break;
- }
-
journal->j_tail_sequence = be32_to_cpu(sb->s_sequence);
journal->j_tail = be32_to_cpu(sb->s_start);
journal->j_first = be32_to_cpu(sb->s_first);
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 1ffcea5c024e..6990fc891612 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -792,11 +792,6 @@ struct journal_s
*/
journal_superblock_t *j_superblock;

- /**
- * @j_format_version: Version of the superblock format.
- */
- int j_format_version;
-
/**
* @j_state_lock: Protect the various scalars in the journal.
*/
--
2.31.1


2023-03-15 08:43:49

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] ext4: Fix reusing stale buffer heads from last failed mounting

On Wed 15-03-23 09:31:23, Zhihao Cheng wrote:
> Following process makes ext4 load stale buffer heads from last failed
> mounting in a new mounting operation:
> mount_bdev
> ext4_fill_super
> | ext4_load_and_init_journal
> | ext4_load_journal
> | jbd2_journal_load
> | load_superblock
> | journal_get_superblock
> | set_buffer_verified(bh) // buffer head is verified
> | jbd2_journal_recover // failed caused by EIO
> | goto failed_mount3a // skip 'sb->s_root' initialization
> deactivate_locked_super
> kill_block_super
> generic_shutdown_super
> if (sb->s_root)
> // false, skip ext4_put_super->invalidate_bdev->
> // invalidate_mapping_pages->mapping_evict_folio->
> // filemap_release_folio->try_to_free_buffers, which
> // cannot drop buffer head.
> blkdev_put
> blkdev_put_whole
> if (atomic_dec_and_test(&bdev->bd_openers))
> // false, systemd-udev happens to open the device. Then
> // blkdev_flush_mapping->kill_bdev->truncate_inode_pages->
> // truncate_inode_folio->truncate_cleanup_folio->
> // folio_invalidate->block_invalidate_folio->
> // filemap_release_folio->try_to_free_buffers will be skipped,
> // dropping buffer head is missed again.
>
> Second mount:
> ext4_fill_super
> ext4_load_and_init_journal
> ext4_load_journal
> ext4_get_journal
> jbd2_journal_init_inode
> journal_init_common
> bh = getblk_unmovable
> bh = __find_get_block // Found stale bh in last failed mounting
> journal->j_sb_buffer = bh
> jbd2_journal_load
> load_superblock
> journal_get_superblock
> if (buffer_verified(bh))
> // true, skip journal->j_format_version = 2, value is 0
> jbd2_journal_recover
> do_one_pass
> next_log_block += count_tags(journal, bh)
> // According to journal_tag_bytes(), 'tag_bytes' calculating is
> // affected by jbd2_has_feature_csum3(), jbd2_has_feature_csum3()
> // returns false because 'j->j_format_version >= 2' is not true,
> // then we get wrong next_log_block. The do_one_pass may exit
> // early whenoccuring non JBD2_MAGIC_NUMBER in 'next_log_block'.
>
> The filesystem is corrupted here, journal is partially replayed, and
> new journal sequence number actually is already used by last mounting.
>
> The invalidate_bdev() can drop all buffer heads even racing with bare
> reading block device(eg. systemd-udev), so we can fix it by invalidating
> bdev in error handling path in __ext4_fill_super().
>
> Fetch a reproducer in [Link].
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217171
> Fixes: 25ed6e8a54df ("jbd2: enable journal clients to enable v2 checksumming")
> Cc: [email protected] # v3.5
> Signed-off-by: Zhihao Cheng <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/super.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index f43e526112ae..61511b7ba017 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1126,6 +1126,12 @@ static void ext4_blkdev_remove(struct ext4_sb_info *sbi)
> struct block_device *bdev;
> bdev = sbi->s_journal_bdev;
> if (bdev) {
> + /*
> + * Invalidate the journal device's buffers. We don't want them
> + * floating about in memory - the physical journal device may
> + * hotswapped, and it breaks the `ro-after' testing code.
> + */
> + invalidate_bdev(bdev);
> ext4_blkdev_put(bdev);
> sbi->s_journal_bdev = NULL;
> }
> @@ -1272,13 +1278,7 @@ static void ext4_put_super(struct super_block *sb)
> sync_blockdev(sb->s_bdev);
> invalidate_bdev(sb->s_bdev);
> if (sbi->s_journal_bdev && sbi->s_journal_bdev != sb->s_bdev) {
> - /*
> - * Invalidate the journal device's buffers. We don't want them
> - * floating about in memory - the physical journal device may
> - * hotswapped, and it breaks the `ro-after' testing code.
> - */
> sync_blockdev(sbi->s_journal_bdev);
> - invalidate_bdev(sbi->s_journal_bdev);
> ext4_blkdev_remove(sbi);
> }
>
> @@ -5610,6 +5610,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> brelse(sbi->s_sbh);
> ext4_blkdev_remove(sbi);
> out_fail:
> + invalidate_bdev(sb->s_bdev);
> sb->s_fs_info = NULL;
> return err ? err : ret;
> }
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-03-15 08:44:24

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v3 2/6] ext4: ext4_put_super: Remove redundant checking for 'sbi->s_journal_bdev'

On Wed 15-03-23 09:31:24, Zhihao Cheng wrote:
> As discussed in [1], 'sbi->s_journal_bdev != sb->s_bdev' will always
> become true if sbi->s_journal_bdev exists. Filesystem block device and
> journal block device are both opened with 'FMODE_EXCL' mode, so these
> two devices can't be same one. Then we can remove the redundant checking
> 'sbi->s_journal_bdev != sb->s_bdev' if 'sbi->s_journal_bdev' exists.
>
> [1] https://lore.kernel.org/lkml/[email protected]/
>
> Signed-off-by: Zhihao Cheng <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ext4/super.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 61511b7ba017..a22417d113ca 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1277,7 +1277,7 @@ static void ext4_put_super(struct super_block *sb)
>
> sync_blockdev(sb->s_bdev);
> invalidate_bdev(sb->s_bdev);
> - if (sbi->s_journal_bdev && sbi->s_journal_bdev != sb->s_bdev) {
> + if (sbi->s_journal_bdev) {
> sync_blockdev(sbi->s_journal_bdev);
> ext4_blkdev_remove(sbi);
> }
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-06-12 01:47:22

by Zhihao Cheng

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] ext4: Fix stale buffer loading from last failed

?? 2023/6/11 12:42, Theodore Ts'o ะด??:
> On Fri, Jun 09, 2023 at 04:04:47PM +0800, Zhihao Cheng wrote:
>>
>> Hi Ted, will this patchset be merged in next window?
>
> It's currently in the dev branch. I haven't set the ack out for it
> yet because I'm still seeing some test failures, including some test
> hangs in my regression tests. There are a number of patches series
> submission that I'm currently working through, so we'll have to see
> what the "guilty" patch set might be, and whether there is an obvious
> fix for it or not. (I've found one such problem that was missed by
> code review[1], and unfortunately, there is at least one more issue
> that I'm currently trying to pin down.)
>
> [1] https://lore.kernel.org/r/[email protected]
>
> Cheers,
>
> - Ted
>
> .
>

Thanks for reminding. now I'm sure this patchset is tracked. Please
follow your process.