[PATCH 1/2] ext4: update s_overhead_clusters in the superblock during an on-line resize

[PATCH 1/2] ext4: update s_overhead_clusters in the superblock during an on-line resize

[Theodore Ts'o]

When doing an online resize, the on-disk superblock on-disk wasn't
updated.  This means that when the file system is unmounted and
remounted, and the on-disk overhead value is non-zero, this would
result in the results of statfs(2) to be incorrect.

This was partially fixed by Commits 10b01ee92df5 ("ext4: fix overhead
calculation to account for the reserved gdt blocks"), 85d825dbf489
("ext4: force overhead calculation if the s_overhead_cluster makes no
sense"), and eb7054212eac ("ext4: update the cached overhead value in
the superblock").

However, since it was too expensive to forcibly recalculate the
overhead for bigalloc file systems at every mount, this didn't fix the
problem for bigalloc file systems.  This commit should address the
problem when resizing file systems with the bigalloc feature enabled.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@kernel.org
---
 fs/ext4/resize.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 8b70a4701293..e5c2713aa11a 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1484,6 +1484,7 @@ static void ext4_update_super(struct super_block *sb,
 	 * Update the fs overhead information
 	 */
 	ext4_calculate_overhead(sb);
+	es->s_overhead_clusters = cpu_to_le32(sbi->s_overhead);
 
 	if (test_opt(sb, DEBUG))
 		printk(KERN_DEBUG "EXT4-fs: added group %u:"
-- 
2.31.0

[PATCH 2/2] ext4: update the s_overhead_clusters in the backup sb's when resizing

[Theodore Ts'o]

When the EXT4_IOC_RESIZE_FS ioctl is complete, update the backup
superblocks.  We don't do this for the old-style resize ioctls since
they are quite ancient, and only used by very old versions of
resize2fs --- and we don't want to update the backup superblocks every
time EXT4_IOC_GROUP_ADD is called, since it might get called a lot.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/ext4.h   |  4 ++--
 fs/ext4/ioctl.c  | 22 +++++++++++++++-------
 fs/ext4/resize.c |  6 +++++-
 fs/ext4/super.c  |  2 +-
 4 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 75b8d81b2469..826fcd12e393 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3016,7 +3016,7 @@ int ext4_fileattr_set(struct user_namespace *mnt_userns,
 		      struct dentry *dentry, struct fileattr *fa);
 int ext4_fileattr_get(struct dentry *dentry, struct fileattr *fa);
 extern void ext4_reset_inode_seed(struct inode *inode);
-int ext4_update_overhead(struct super_block *sb);
+int ext4_update_overhead(struct super_block *sb, bool force);
 
 /* migrate.c */
 extern int ext4_ext_migrate(struct inode *);
@@ -3799,7 +3799,7 @@ static inline void set_bitmap_uptodate(struct buffer_head *bh)
 extern wait_queue_head_t ext4__ioend_wq[EXT4_WQ_HASH_SZ];
 
 extern int ext4_resize_begin(struct super_block *sb);
-extern void ext4_resize_end(struct super_block *sb);
+extern int ext4_resize_end(struct super_block *sb, bool update_backups);
 
 static inline void ext4_set_io_unwritten_flag(struct inode *inode,
 					      struct ext4_io_end *io_end)
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index cb01c1da0f9d..1702c574407a 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -944,7 +944,9 @@ static long ext4_ioctl_group_add(struct file *file,
 	    test_opt(sb, INIT_INODE_TABLE))
 		err = ext4_register_li_request(sb, input->group);
 group_add_out:
-	ext4_resize_end(sb);
+	err2 = ext4_resize_end(sb, false);
+	if (err == 0)
+		err = err2;
 	return err;
 }
 
@@ -1223,7 +1225,9 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			err = err2;
 		mnt_drop_write_file(filp);
 group_extend_out:
-		ext4_resize_end(sb);
+		err2 = ext4_resize_end(sb, false);
+		if (err == 0)
+			err = err2;
 		return err;
 	}
 
@@ -1371,7 +1375,9 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			err = ext4_register_li_request(sb, o_group);
 
 resizefs_out:
-		ext4_resize_end(sb);
+		err2 = ext4_resize_end(sb, true);
+		if (err == 0)
+			err = err2;
 		return err;
 	}
 
@@ -1599,13 +1605,15 @@ static void set_overhead(struct ext4_super_block *es, const void *arg)
 	es->s_overhead_clusters = cpu_to_le32(*((unsigned long *) arg));
 }
 
-int ext4_update_overhead(struct super_block *sb)
+int ext4_update_overhead(struct super_block *sb, bool force)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 
-	if (sb_rdonly(sb) || sbi->s_overhead == 0 ||
-	    sbi->s_overhead == le32_to_cpu(sbi->s_es->s_overhead_clusters))
+	if (sb_rdonly(sb))
+		return 0;
+	if (!force &&
+	    (sbi->s_overhead == 0 ||
+	     sbi->s_overhead == le32_to_cpu(sbi->s_es->s_overhead_clusters)))
 		return 0;
-
 	return ext4_update_superblocks_fn(sb, set_overhead, &sbi->s_overhead);
 }
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index e5c2713aa11a..8abff9400f69 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -97,10 +97,14 @@ int ext4_resize_begin(struct super_block *sb)
 	return ret;
 }
 
-void ext4_resize_end(struct super_block *sb)
+int ext4_resize_end(struct super_block *sb, bool update_backups)
 {
 	clear_bit_unlock(EXT4_FLAGS_RESIZING, &EXT4_SB(sb)->s_ext4_flags);
 	smp_mb__after_atomic();
+	if (update_backups)
+		return ext4_update_overhead(sb, true);
+	else
+		return 0;
 }
 
 static ext4_group_t ext4_meta_bg_first_group(struct super_block *sb,
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 845f2f8aee5f..6a8a752d812b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5523,7 +5523,7 @@ static int ext4_fill_super(struct super_block *sb, struct fs_context *fc)
 			 "Quota mode: %s.", descr, ext4_quota_mode(sb));
 
 	/* Update the s_overhead_clusters if necessary */
-	ext4_update_overhead(sb);
+	ext4_update_overhead(sb, false);
 	return 0;
 
 free_sbi:
-- 
2.31.0

Re: [PATCH 2/2] ext4: update the s_overhead_clusters in the backup sb's when resizing

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

On Jun 28, 2022, at 10:00 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> When the EXT4_IOC_RESIZE_FS ioctl is complete, update the backup
> superblocks.  We don't do this for the old-style resize ioctls since
> they are quite ancient, and only used by very old versions of
> resize2fs --- and we don't want to update the backup superblocks every
> time EXT4_IOC_GROUP_ADD is called, since it might get called a lot.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>

One minor style nit below, but you can take it or leave it...

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index e5c2713aa11a..8abff9400f69 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -97,10 +97,14 @@ int ext4_resize_begin(struct super_block *sb)
> 	return ret;
> }
> 
> -void ext4_resize_end(struct super_block *sb)
> +int ext4_resize_end(struct super_block *sb, bool update_backups)
> {
> 	clear_bit_unlock(EXT4_FLAGS_RESIZING, &EXT4_SB(sb)->s_ext4_flags);
> 	smp_mb__after_atomic();
> +	if (update_backups)
> +		return ext4_update_overhead(sb, true);
> +	else
> +		return 0;

(style) no need for "else" after return.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 2/2] ext4: update the s_overhead_clusters in the backup sb's when resizing

[Theodore Ts'o]

On Mon, Jul 04, 2022 at 02:50:47PM -0600, Andreas Dilger wrote:
> > -void ext4_resize_end(struct super_block *sb)
> > +int ext4_resize_end(struct super_block *sb, bool update_backups)
> > {
> > 	clear_bit_unlock(EXT4_FLAGS_RESIZING, &EXT4_SB(sb)->s_ext4_flags);
> > 	smp_mb__after_atomic();
> > +	if (update_backups)
> > +		return ext4_update_overhead(sb, true);
> > +	else
> > +		return 0;
> 
> (style) no need for "else" after return.

Good point, I'll make that adjustment.

				- Ted

Re: [PATCH 1/2] ext4: update s_overhead_clusters in the superblock during an on-line resize

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]

On Jun 28, 2022, at 10:00 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> When doing an online resize, the on-disk superblock on-disk wasn't
> updated.  This means that when the file system is unmounted and
> remounted, and the on-disk overhead value is non-zero, this would
> result in the results of statfs(2) to be incorrect.
> 
> This was partially fixed by Commits 10b01ee92df5 ("ext4: fix overhead
> calculation to account for the reserved gdt blocks"), 85d825dbf489
> ("ext4: force overhead calculation if the s_overhead_cluster makes no
> sense"), and eb7054212eac ("ext4: update the cached overhead value in
> the superblock").

Would these be better referenced by Fixes: labels?

> However, since it was too expensive to forcibly recalculate the
> overhead for bigalloc file systems at every mount, this didn't fix the
> problem for bigalloc file systems.  This commit should address the
> problem when resizing file systems with the bigalloc feature enabled.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Cc: stable@kernel.org

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/ext4/resize.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 8b70a4701293..e5c2713aa11a 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -1484,6 +1484,7 @@ static void ext4_update_super(struct super_block *sb,
> 	 * Update the fs overhead information
> 	 */
> 	ext4_calculate_overhead(sb);
> +	es->s_overhead_clusters = cpu_to_le32(sbi->s_overhead);
> 
> 	if (test_opt(sb, DEBUG))
> 		printk(KERN_DEBUG "EXT4-fs: added group %u:"
> --
> 2.31.0
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 1/2] ext4: update s_overhead_clusters in the superblock during an on-line resize

[Theodore Ts'o]

On Mon, Jul 04, 2022 at 02:47:43PM -0600, Andreas Dilger wrote:
> On Jun 28, 2022, at 10:00 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> > 
> > When doing an online resize, the on-disk superblock on-disk wasn't
> > updated.  This means that when the file system is unmounted and
> > remounted, and the on-disk overhead value is non-zero, this would
> > result in the results of statfs(2) to be incorrect.
> > 
> > This was partially fixed by Commits 10b01ee92df5 ("ext4: fix overhead
> > calculation to account for the reserved gdt blocks"), 85d825dbf489
> > ("ext4: force overhead calculation if the s_overhead_cluster makes no
> > sense"), and eb7054212eac ("ext4: update the cached overhead value in
> > the superblock").
> 
> Would these be better referenced by Fixes: labels?

This commit doesn't actually _fix_ the above-mentioned commits.  They
just didn't fix the bug which is addressed by this one.

Cheers,

						- Ted

Re: [PATCH 1/2] ext4: update s_overhead_clusters in the superblock during an on-line resize

[Theodore Ts'o]

On Wed, 29 Jun 2022 00:00:25 -0400, Theodore Ts'o wrote:
> When doing an online resize, the on-disk superblock on-disk wasn't
> updated.  This means that when the file system is unmounted and
> remounted, and the on-disk overhead value is non-zero, this would
> result in the results of statfs(2) to be incorrect.
> 
> This was partially fixed by Commits 10b01ee92df5 ("ext4: fix overhead
> calculation to account for the reserved gdt blocks"), 85d825dbf489
> ("ext4: force overhead calculation if the s_overhead_cluster makes no
> sense"), and eb7054212eac ("ext4: update the cached overhead value in
> the superblock").
> 
> [...]

Applied, thanks!

[1/2] ext4: update s_overhead_clusters in the superblock during an on-line resize
      commit: e781b8ce9261f353df91b94303e53c31fdf9871e
[2/2] ext4: update the s_overhead_clusters in the backup sb's when resizing
      commit: 2c8204b83ceaf439dff2d1a94a7e2d3ad7619287

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>