ext4: don't update s_rev_level if not required

Message ID 1547339800-50257-1-git-send-email-adilger@dilger.ca
State New
Headers show
Series
  • ext4: don't update s_rev_level if not required
Related show

Commit Message

Andreas Dilger Jan. 13, 2019, 12:36 a.m.
Don't update the superblock s_rev_level during mount if it isn't
actually necessary, only if superblock features are being set by
the kernel.  This was originally added for ext3 since it always
set the INCOMPAT_RECOVER and HAS_JOURNAL features during mount,
but this is not needed since no journal mode was added to ext4.

That will allow Geert to mount his 20-year-old ext2 rev 0.0 m68k
filesystem, as a testament of the backward compatibility of ext4.

Fixes: 0390131ba84f ("ext4: Allow ext4 to run without a journal")
Signed-off-by: Andreas Dilger <adilger@dilger.ca>
---
 fs/ext4/ext4.h  | 3 +++
 fs/ext4/inode.c | 1 -
 fs/ext4/super.c | 1 -
 3 files changed, 3 insertions(+), 2 deletions(-)

Comments

Geert Uytterhoeven Jan. 14, 2019, 10:25 a.m. | #1
Hi Andreas,

On Sun, Jan 13, 2019 at 1:36 AM Andreas Dilger <adilger@dilger.ca> wrote:
> Don't update the superblock s_rev_level during mount if it isn't
> actually necessary, only if superblock features are being set by
> the kernel.  This was originally added for ext3 since it always
> set the INCOMPAT_RECOVER and HAS_JOURNAL features during mount,
> but this is not needed since no journal mode was added to ext4.
>
> That will allow Geert to mount his 20-year-old ext2 rev 0.0 m68k
> filesystem, as a testament of the backward compatibility of ext4.
>
> Fixes: 0390131ba84f ("ext4: Allow ext4 to run without a journal")
> Signed-off-by: Andreas Dilger <adilger@dilger.ca>

Thanks for your patch!

> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1670,6 +1670,7 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
>  } \
>  static inline void ext4_set_feature_##name(struct super_block *sb) \
>  { \
> +       ext4_update_dynamic_rev(sb); \

This doesn't compile, as ext4_update_dynamic_rev() is used before its
declaration:

In file included from fs/ext4/bitmap.c:12:0:
fs/ext4/ext4.h: In function ‘ext4_set_feature_dir_prealloc’:
fs/ext4/ext4.h:1673:2: error: implicit declaration of function
‘ext4_update_dynamic_rev’ [-Werror=implicit-function-declaration]
  ext4_update_dynamic_rev(sb); \
  ^
fs/ext4/ext4.h:1719:1: note: in expansion of macro ‘EXT4_FEATURE_COMPAT_FUNCS’
 EXT4_FEATURE_COMPAT_FUNCS(dir_prealloc,  DIR_PREALLOC)
 ^~~~~~~~~~~~~~~~~~~~~~~~~
fs/ext4/ext4.h: At top level:
fs/ext4/ext4.h:2678:13: warning: conflicting types for ‘ext4_update_dynamic_rev’
 extern void ext4_update_dynamic_rev(struct super_block *sb);
             ^~~~~~~~~~~~~~~~~~~~~~~
fs/ext4/ext4.h:1673:2: note: previous implicit declaration of
‘ext4_update_dynamic_rev’ was here
  ext4_update_dynamic_rev(sb); \
  ^
fs/ext4/ext4.h:1719:1: note: in expansion of macro ‘EXT4_FEATURE_COMPAT_FUNCS’
 EXT4_FEATURE_COMPAT_FUNCS(dir_prealloc,  DIR_PREALLOC)
 ^~~~~~~~~~~~~~~~~~~~~~~~~

Moving the declaration up fixes that.

Hence with that fixed:
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Theodore Y. Ts'o Feb. 15, 2019, 2:10 a.m. | #2
On Sat, Jan 12, 2019 at 05:36:40PM -0700, Andreas Dilger wrote:
> Don't update the superblock s_rev_level during mount if it isn't
> actually necessary, only if superblock features are being set by
> the kernel.  This was originally added for ext3 since it always
> set the INCOMPAT_RECOVER and HAS_JOURNAL features during mount,
> but this is not needed since no journal mode was added to ext4.
> 
> That will allow Geert to mount his 20-year-old ext2 rev 0.0 m68k
> filesystem, as a testament of the backward compatibility of ext4.
> 
> Fixes: 0390131ba84f ("ext4: Allow ext4 to run without a journal")
> Signed-off-by: Andreas Dilger <adilger@dilger.ca>

Thanks, applied.  I had to move the declaration of
ext4_update_dynamic_rev() earlier as Geert suggested.

						- Ted

Patch

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 185a05d..a1357f3 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1670,6 +1670,7 @@  static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
 } \
 static inline void ext4_set_feature_##name(struct super_block *sb) \
 { \
+	ext4_update_dynamic_rev(sb); \
 	EXT4_SB(sb)->s_es->s_feature_compat |= \
 		cpu_to_le32(EXT4_FEATURE_COMPAT_##flagname); \
 } \
@@ -1687,6 +1688,7 @@  static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
 } \
 static inline void ext4_set_feature_##name(struct super_block *sb) \
 { \
+	ext4_update_dynamic_rev(sb); \
 	EXT4_SB(sb)->s_es->s_feature_ro_compat |= \
 		cpu_to_le32(EXT4_FEATURE_RO_COMPAT_##flagname); \
 } \
@@ -1704,6 +1706,7 @@  static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
 } \
 static inline void ext4_set_feature_##name(struct super_block *sb) \
 { \
+	ext4_update_dynamic_rev(sb); \
 	EXT4_SB(sb)->s_es->s_feature_incompat |= \
 		cpu_to_le32(EXT4_FEATURE_INCOMPAT_##flagname); \
 } \
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 34d7e07..f21f2ac 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5351,7 +5351,6 @@  static int ext4_do_update_inode(handle_t *handle,
 		err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
 		if (err)
 			goto out_brelse;
-		ext4_update_dynamic_rev(sb);
 		ext4_set_feature_large_file(sb);
 		ext4_handle_sync(handle);
 		err = ext4_handle_dirty_super(handle, sb);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index fb12d3c..6e4cac6 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2249,7 +2249,6 @@  static int ext4_setup_super(struct super_block *sb, struct ext4_super_block *es,
 		es->s_max_mnt_count = cpu_to_le16(EXT4_DFL_MAX_MNT_COUNT);
 	le16_add_cpu(&es->s_mnt_count, 1);
 	ext4_update_tstamp(es, s_mtime);
-	ext4_update_dynamic_rev(sb);
 	if (sbi->s_journal)
 		ext4_set_feature_journal_needs_recovery(sb);