diff mbox

mke2fs: disable resize_inode feature if 64bit feature is enabled

Message ID 1372928710-10461-1-git-send-email-guaneryu@gmail.com
State Rejected, archived
Headers show

Commit Message

Eryu Guan July 4, 2013, 9:05 a.m. UTC
Since auto_64-bit_support is on by default, resize_inode feature will
be disabled when creating a >16T ext4 according to mke2fs.conf(5).

This should also be done when making ext4 with "-O 64bit" to enable
64bit feature explicitly. Otherwise online resize to enlarge a
over-16T fs to larger would fail.

[root@localhost resize]# truncate -s 50t fs.img
[root@localhost resize]# losetup /dev/loop0 fs.img
[root@localhost resize]# mkfs -t ext4 -O 64bit /dev/loop0 30t
[root@localhost resize]# mount /dev/loop0 mnt
[root@localhost resize]# resize2fs /dev/loop0
resize2fs 1.42.7 (21-Jan-2013)
Filesystem at /dev/loop0 is mounted on /root/resize/mnt; on-line resizing required
old_desc_blocks = 3840, new_desc_blocks = 6400
resize2fs: Invalid argument While checking for on-line resizing support

And dmesg shows
[688378.442623] EXT4-fs (loop0): resizing filesystem from 6710886400 to 13421772800 blocks
[688378.443216] EXT4-fs warning (device loop0): verify_reserved_gdb:700: reserved GDT 3201 missing grp 177147 (5804756097)
[688378.443222] EXT4-fs (loop0): resized filesystem to 8858370048
[688378.528451] EXT4-fs warning (device loop0): ext4_group_extend:1710: can't shrink FS - resize aborted

With this fix resize2fs could do the online enlarge correctly.

Signed-off-by: Eryu Guan <guaneryu@gmail.com>
---
 misc/mke2fs.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Zheng Liu July 5, 2013, 1:21 a.m. UTC | #1
On Thu, Jul 04, 2013 at 05:05:10PM +0800, Eryu Guan wrote:
> Since auto_64-bit_support is on by default, resize_inode feature will
> be disabled when creating a >16T ext4 according to mke2fs.conf(5).
> 
> This should also be done when making ext4 with "-O 64bit" to enable
> 64bit feature explicitly. Otherwise online resize to enlarge a
> over-16T fs to larger would fail.
> 
> [root@localhost resize]# truncate -s 50t fs.img
> [root@localhost resize]# losetup /dev/loop0 fs.img
> [root@localhost resize]# mkfs -t ext4 -O 64bit /dev/loop0 30t
> [root@localhost resize]# mount /dev/loop0 mnt
> [root@localhost resize]# resize2fs /dev/loop0
> resize2fs 1.42.7 (21-Jan-2013)
> Filesystem at /dev/loop0 is mounted on /root/resize/mnt; on-line resizing required
> old_desc_blocks = 3840, new_desc_blocks = 6400
> resize2fs: Invalid argument While checking for on-line resizing support
> 
> And dmesg shows
> [688378.442623] EXT4-fs (loop0): resizing filesystem from 6710886400 to 13421772800 blocks
> [688378.443216] EXT4-fs warning (device loop0): verify_reserved_gdb:700: reserved GDT 3201 missing grp 177147 (5804756097)
> [688378.443222] EXT4-fs (loop0): resized filesystem to 8858370048
> [688378.528451] EXT4-fs warning (device loop0): ext4_group_extend:1710: can't shrink FS - resize aborted
> 
> With this fix resize2fs could do the online enlarge correctly.
> 
> Signed-off-by: Eryu Guan <guaneryu@gmail.com>
> ---
>  misc/mke2fs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index d96f156..e4102b9 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -1790,6 +1790,9 @@ profile_error:
>  		fs_param.s_log_block_size = 2;
>  	}
>  	if ((fs_blocks_count > MAX_32_NUM) &&

Do we really need to check it?  My point is that we just clear
RESIZE_INODE flag if 64BIT flag is enabled.

                                                - Zheng

> +	    (fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT))
> +		fs_param.s_feature_compat &= ~EXT2_FEATURE_COMPAT_RESIZE_INODE;
> +	if ((fs_blocks_count > MAX_32_NUM) &&
>  	    !(fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT) &&
>  	    get_bool_from_profile(fs_types, "auto_64-bit_support", 0)) {
>  		fs_param.s_feature_incompat |= EXT4_FEATURE_INCOMPAT_64BIT;
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o July 7, 2013, 3:41 p.m. UTC | #2
On Fri, Jul 05, 2013 at 09:21:28AM +0800, Zheng Liu wrote:
> Do we really need to check it?  My point is that we just clear
> RESIZE_INODE flag if 64BIT flag is enabled.

It's better to use the resize_inode for file systems that are smaller
than 16TB, even if the 64-bit flag is enabled, since using the meta_bg
style resizing spreads out the block group descriptors, which
increases the time to mount the file system.  Using the resize_inode
will defer the need to go to the meta_bg-style resizing until we cross
the 16TB boundary.

Cheers,

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukas Czerner March 3, 2014, 3:40 p.m. UTC | #3
On Sun, 7 Jul 2013, Theodore Ts'o wrote:

> Date: Sun, 7 Jul 2013 11:41:40 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Eryu Guan <guaneryu@gmail.com>, linux-ext4@vger.kernel.org
> Subject: Re: [PATCH] mke2fs: disable resize_inode feature if 64bit feature is
>     enabled

Hi Ted,

are you going to take this in ?

Thanks!
-Lukas

> 
> On Fri, Jul 05, 2013 at 09:21:28AM +0800, Zheng Liu wrote:
> > Do we really need to check it?  My point is that we just clear
> > RESIZE_INODE flag if 64BIT flag is enabled.
> 
> It's better to use the resize_inode for file systems that are smaller
> than 16TB, even if the 64-bit flag is enabled, since using the meta_bg
> style resizing spreads out the block group descriptors, which
> increases the time to mount the file system.  Using the resize_inode
> will defer the need to go to the meta_bg-style resizing until we cross
> the 16TB boundary.
> 
> Cheers,
> 
> 						- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o March 4, 2014, 3:19 p.m. UTC | #4
On Mon, Mar 03, 2014 at 04:40:21PM +0100, Lukáš Czerner wrote:
> 
> are you going to take this in ?
> 
> > It's better to use the resize_inode for file systems that are smaller
> > than 16TB, even if the 64-bit flag is enabled, since using the meta_bg
> > style resizing spreads out the block group descriptors, which
> > increases the time to mount the file system.  Using the resize_inode
> > will defer the need to go to the meta_bg-style resizing until we cross
> > the 16TB boundary.

See my comments above.

If we create a file system which is say, 1T but with the 64-bit flag,
we do want the resize_inode feature to be used until we cross over the
32-bit boundary, because keeping block group descriptors contiguous
speeds up the mount time.

If I were to accept this patch, and/or Red Hat were to ship with it, I
will predict that you will get angry customer support calls about file
system mount times getting slow after doing a resize.

At some point what we may want to lazily load the block group
descriptors at mount time, to mitigate the slow mount time issue when
using meta_bg.  We have a lot of assumptions in the code that the
block group descriptors are always available, though.

      	    		    	   	      - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukas Czerner March 6, 2014, 2:03 p.m. UTC | #5
On Tue, 4 Mar 2014, Theodore Ts'o wrote:

> Date: Tue, 4 Mar 2014 10:19:25 -0500
> From: Theodore Ts'o <tytso@mit.edu>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: Eryu Guan <guaneryu@gmail.com>, linux-ext4@vger.kernel.org
> Subject: Re: [PATCH] mke2fs: disable resize_inode feature if 64bit feature is
>     enabled
> 
> On Mon, Mar 03, 2014 at 04:40:21PM +0100, Lukáš Czerner wrote:
> > 
> > are you going to take this in ?
> > 
> > > It's better to use the resize_inode for file systems that are smaller
> > > than 16TB, even if the 64-bit flag is enabled, since using the meta_bg
> > > style resizing spreads out the block group descriptors, which
> > > increases the time to mount the file system.  Using the resize_inode
> > > will defer the need to go to the meta_bg-style resizing until we cross
> > > the 16TB boundary.
> 
> See my comments above.
> 
> If we create a file system which is say, 1T but with the 64-bit flag,
> we do want the resize_inode feature to be used until we cross over the
> 32-bit boundary, because keeping block group descriptors contiguous
> speeds up the mount time.
> 
> If I were to accept this patch, and/or Red Hat were to ship with it, I
> will predict that you will get angry customer support calls about file
> system mount times getting slow after doing a resize.
> 
> At some point what we may want to lazily load the block group
> descriptors at mount time, to mitigate the slow mount time issue when
> using meta_bg.  We have a lot of assumptions in the code that the
> block group descriptors are always available, though.
> 
>       	    		    	   	      - Ted

Hi Ted,

I am sorry, but am I missing something ? This patch seems to do
exactly that.

	if ((fs_blocks_count > MAX_32_NUM) &&
+	    (fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT))
+		fs_param.s_feature_compat &= ~EXT2_FEATURE_COMPAT_RESIZE_INODE;

The answer from you on that patch is commenting on a guestion from
Zheng Liu who is asking whether we could skip the MAX_32_NUM check.

So the patch seems to be ok for me.

Thanks!
-Lukas
Theodore Ts'o March 6, 2014, 3:18 p.m. UTC | #6
On Thu, Mar 06, 2014 at 03:03:30PM +0100, Lukáš Czerner wrote:
> 
> I am sorry, but am I missing something ? This patch seems to do
> exactly that.
> 
> 	if ((fs_blocks_count > MAX_32_NUM) &&
> +	    (fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT))
> +		fs_param.s_feature_compat &= ~EXT2_FEATURE_COMPAT_RESIZE_INODE;
> 
> The answer from you on that patch is commenting on a guestion from
> Zheng Liu who is asking whether we could skip the MAX_32_NUM check.

You're right, I misread the patch and the e-mail thread.

Thanks, I've applied the patch.

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index d96f156..e4102b9 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -1790,6 +1790,9 @@  profile_error:
 		fs_param.s_log_block_size = 2;
 	}
 	if ((fs_blocks_count > MAX_32_NUM) &&
+	    (fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT))
+		fs_param.s_feature_compat &= ~EXT2_FEATURE_COMPAT_RESIZE_INODE;
+	if ((fs_blocks_count > MAX_32_NUM) &&
 	    !(fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT) &&
 	    get_bool_from_profile(fs_types, "auto_64-bit_support", 0)) {
 		fs_param.s_feature_incompat |= EXT4_FEATURE_INCOMPAT_64BIT;