On Fri 10-03-23 20:52:04, Zhihao Cheng wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>
> 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 <yi.zhang@xxxxxxxxxx>
> Signed-off-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> 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 e80c781731f8..b991d5c21d16 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -2059,10 +2059,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 &
> @@ -2224,7 +2226,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;
> @@ -2254,11 +2256,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 ad7bb6861143..7095c0f17ad0 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1305,11 +1305,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); \
> } \
> @@ -1327,7 +1338,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); \
> } \
> @@ -1345,7 +1356,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
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
