[PATCH 1/2] fs: ext4: Consolidate checks for resize of bigalloc into ext4_resize_begin

[PATCH 1/2] fs: ext4: Consolidate checks for resize of bigalloc into ext4_resize_begin

[Josh Triplett]

Two different places checked for attempts to resize a filesystem with
the bigalloc feature. Move the check into ext4_resize_begin, which both
places already call.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 fs/ext4/ioctl.c  | 14 --------------
 fs/ext4/resize.c |  5 +++++
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 31627f7dc5cd..48a7aace337c 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -693,13 +693,6 @@ static long ext4_ioctl_group_add(struct file *file,
 	if (err)
 		return err;
 
-	if (ext4_has_feature_bigalloc(sb)) {
-		ext4_msg(sb, KERN_ERR,
-			 "Online resizing not supported with bigalloc");
-		err = -EOPNOTSUPP;
-		goto group_add_out;
-	}
-
 	err = mnt_want_write_file(file);
 	if (err)
 		goto group_add_out;
@@ -871,13 +864,6 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			goto group_extend_out;
 		}
 
-		if (ext4_has_feature_bigalloc(sb)) {
-			ext4_msg(sb, KERN_ERR,
-				 "Online resizing not supported with bigalloc");
-			err = -EOPNOTSUPP;
-			goto group_extend_out;
-		}
-
 		err = mnt_want_write_file(filp);
 		if (err)
 			goto group_extend_out;
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index bd0d185654f3..d13bb9e76482 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -74,6 +74,11 @@ int ext4_resize_begin(struct super_block *sb)
 		return -EPERM;
 	}
 
+	if (ext4_has_feature_bigalloc(sb)) {
+		ext4_msg(sb, KERN_ERR, "Online resizing not supported with bigalloc");
+		return -EOPNOTSUPP;
+	}
+
 	if (test_and_set_bit_lock(EXT4_FLAGS_RESIZING,
 				  &EXT4_SB(sb)->s_ext4_flags))
 		ret = -EBUSY;
-- 
2.32.0.rc2

[PATCH 2/2] fs: ext4: Add check to prevent attempting to resize an fs with sparse_super2

[Josh Triplett]

The in-kernel ext4 resize code doesn't support filesystem with the
sparse_super2 feature. It fails with errors like this and doesn't finish
the resize:
EXT4-fs (loop0): resizing filesystem from 16640 to 7864320 blocks
EXT4-fs warning (device loop0): verify_reserved_gdb:760: reserved GDT 2 missing grp 1 (32770)
EXT4-fs warning (device loop0): ext4_resize_fs:2111: error (-22) occurred during file system resize
EXT4-fs (loop0): resized filesystem to 2097152

To reproduce:
mkfs.ext4 -b 4096 -I 256 -J size=32 -E resize=$((256*1024*1024)) -O sparse_super2 ext4.img 65M
truncate -s 30G ext4.img
mount ext4.img /mnt
python3 -c 'import fcntl, os, struct ; fd = os.open("/mnt", os.O_RDONLY | os.O_DIRECTORY) ; fcntl.ioctl(fd, 0x40086610, struct.pack("Q", 30 * 1024 * 1024 * 1024 // 4096), False) ; os.close(fd)'
dmesg | tail
e2fsck ext4.img

The userspace resize2fs tool has a check for this case: it checks if the
filesystem has sparse_super2 set and if the kernel provides
/sys/fs/ext4/features/sparse_super2. However, the former check requires
manually reading and parsing the filesystem superblock.

Detect this case in ext4_resize_begin and error out early with a clear
error message.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 fs/ext4/resize.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index d13bb9e76482..fc885914c88a 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -78,6 +78,10 @@ int ext4_resize_begin(struct super_block *sb)
 		ext4_msg(sb, KERN_ERR, "Online resizing not supported with bigalloc");
 		return -EOPNOTSUPP;
 	}
+	if (ext4_has_feature_sparse_super2(sb)) {
+		ext4_msg(sb, KERN_ERR, "Online resizing not supported with sparse_super2");
+		return -EOPNOTSUPP;
+	}
 
 	if (test_and_set_bit_lock(EXT4_FLAGS_RESIZING,
 				  &EXT4_SB(sb)->s_ext4_flags))
-- 
2.32.0.rc2

Re: [PATCH 1/2] fs: ext4: Consolidate checks for resize of bigalloc into ext4_resize_begin

[Theodore Ts'o]

On Mon, Jun 07, 2021 at 12:15:08PM -0700, Josh Triplett wrote:
> Two different places checked for attempts to resize a filesystem with
> the bigalloc feature. Move the check into ext4_resize_begin, which both
> places already call.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Applied, thanks.

					- Ted

Re: [PATCH 2/2] fs: ext4: Add check to prevent attempting to resize an fs with sparse_super2

[Theodore Ts'o]

On Mon, Jun 07, 2021 at 12:15:24PM -0700, Josh Triplett wrote:
> The in-kernel ext4 resize code doesn't support filesystem with the
> sparse_super2 feature. It fails with errors like this and doesn't finish
> the resize:
> EXT4-fs (loop0): resizing filesystem from 16640 to 7864320 blocks
> EXT4-fs warning (device loop0): verify_reserved_gdb:760: reserved GDT 2 missing grp 1 (32770)
> EXT4-fs warning (device loop0): ext4_resize_fs:2111: error (-22) occurred during file system resize
> EXT4-fs (loop0): resized filesystem to 2097152
> 
> To reproduce:
> mkfs.ext4 -b 4096 -I 256 -J size=32 -E resize=$((256*1024*1024)) -O sparse_super2 ext4.img 65M
> truncate -s 30G ext4.img
> mount ext4.img /mnt
> python3 -c 'import fcntl, os, struct ; fd = os.open("/mnt", os.O_RDONLY | os.O_DIRECTORY) ; fcntl.ioctl(fd, 0x40086610, struct.pack("Q", 30 * 1024 * 1024 * 1024 // 4096), False) ; os.close(fd)'
> dmesg | tail
> e2fsck ext4.img
> 
> The userspace resize2fs tool has a check for this case: it checks if the
> filesystem has sparse_super2 set and if the kernel provides
> /sys/fs/ext4/features/sparse_super2. However, the former check requires
> manually reading and parsing the filesystem superblock.
> 
> Detect this case in ext4_resize_begin and error out early with a clear
> error message.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Applied, thanks.

					- Ted

Re: [PATCH 1/2] fs: ext4: Consolidate checks for resize of bigalloc into ext4_resize_begin

[Theodore Ts'o]

On Thu, Jun 24, 2021 at 10:23:15AM -0400, Theodore Ts'o wrote:
> On Mon, Jun 07, 2021 at 12:15:08PM -0700, Josh Triplett wrote:
> > Two different places checked for attempts to resize a filesystem with
> > the bigalloc feature. Move the check into ext4_resize_begin, which both
> > places already call.
> > 
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> 
> Applied, thanks.

I'm going to have to revert this change, since it breaks online
resizing for bigalloc file system.  The issue is that
ext4_resize_begin() is called from *three* places: for
EXT4_IOC_GROUP_ADD, EXT4_IOC_GROUP_EXTEND, and EXT4_IOC_RESIZE_FS.
The first two ioctls are used for the "old-style" online resize, and
bigalloc is not supported for those two ioctls.  However, it *is*
supposed to work for EXT4_IOC_RESIZE_FS.

Unfortunately, this just caused some tests to be skipped (assuming
that this was an old kernel that didn't support this feature) and I
didn't notice it right away.

						- Ted

Re: [PATCH 1/2] fs: ext4: Consolidate checks for resize of bigalloc into ext4_resize_begin

[Josh Triplett]

On Wed, Jun 30, 2021 at 08:48:04PM -0400, Theodore Ts'o wrote:
> On Thu, Jun 24, 2021 at 10:23:15AM -0400, Theodore Ts'o wrote:
> > On Mon, Jun 07, 2021 at 12:15:08PM -0700, Josh Triplett wrote:
> > > Two different places checked for attempts to resize a filesystem with
> > > the bigalloc feature. Move the check into ext4_resize_begin, which both
> > > places already call.
> > > 
> > > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > 
> > Applied, thanks.
> 
> I'm going to have to revert this change, since it breaks online
> resizing for bigalloc file system.  The issue is that
> ext4_resize_begin() is called from *three* places: for
> EXT4_IOC_GROUP_ADD, EXT4_IOC_GROUP_EXTEND, and EXT4_IOC_RESIZE_FS.
> The first two ioctls are used for the "old-style" online resize, and
> bigalloc is not supported for those two ioctls.  However, it *is*
> supposed to work for EXT4_IOC_RESIZE_FS.
> 
> Unfortunately, this just caused some tests to be skipped (assuming
> that this was an old kernel that didn't support this feature) and I
> didn't notice it right away.

Ah, I see. I didn't realize that resizing bigalloc was possible with
EXT4_IOC_RESIZE_FS; I'd assumed (incorrectly) from the error message
that it wasn't.

This patch was *purely* a side story of the second patch. I'd discovered
that the kernel couldn't successfully resize a filesystem with
sparse_super2, and wanted to catch that in the kernel and return a clear
error, rather than partially resizing the filesystem. In the course of
making that change, I noticed the two copies of the error for the
bigalloc case and tried to consolidate them.

Sorry to have missed the third case here, and no problems with the
revert. I'm hoping that the second patch can be kept as-is, assuming
there's no support for resizing sparse_super2 by any code path?

- Josh Triplett

Re: [PATCH 1/2] fs: ext4: Consolidate checks for resize of bigalloc into ext4_resize_begin

[Theodore Ts'o]

On Wed, Jun 30, 2021 at 07:17:50PM -0700, Josh Triplett wrote:
> Sorry to have missed the third case here, and no problems with the
> revert. I'm hoping that the second patch can be kept as-is, assuming
> there's no support for resizing sparse_super2 by any code path?

No worries, I missed it too in my review.  Yes, I only reverted the
first patch, and kept the second one.  There was a minor conflict in
the revert, but it was easily fixed up.

    	    	       	      	    - Ted