2008-11-22 15:02:51

by Eric Sandeen

[permalink] [raw]
Subject: [PATCH] e2fsck: ignore differing NEEDS_RECOVERY flag on backup sbs

This is for RH bugzilla 471925 - Complete scan of filesystems expanded online

When we resize online, the primary superblock gets copied to all
the backups, and of course since we're mounted the NEEDS_RECOVERY
flag is set. A subsequent fsck will find the backups have the
NEEDS_RECOVERY flag set while the primary does not, and this
forces a full fsck pass.

I think this flag can be safely ignored in the flag comparisons.

Signed-off-by: Eric Sandeen <[email protected]>
---

Index: e2fsprogs/e2fsck/super.c
===================================================================
--- e2fsprogs.orig/e2fsck/super.c 2008-11-22 07:54:47.000000000 -0600
+++ e2fsprogs/e2fsck/super.c 2008-11-22 08:59:45.953060973 -0600
@@ -860,7 +860,8 @@ void check_super_block(e2fsck_t ctx)
* try to discourage it in the future. In particular, for the newer
* ext4 files, especially EXT4_FEATURE_RO_COMPAT_DIR_NLINK and
* EXT3_FEATURE_INCOMPAT_EXTENTS. So some of these may go away in the
- * future.
+ * future. EXT3_FEATURE_INCOMPAT_RECOVER may also get set when
+ * copying the primary superblock during online resize.
*
* The kernel will set EXT2_FEATURE_COMPAT_EXT_ATTR, but
* unfortunately, we shouldn't ignore it since if it's not set in the
@@ -869,7 +870,8 @@ void check_super_block(e2fsck_t ctx)
*/
#define FEATURE_RO_COMPAT_IGNORE (EXT2_FEATURE_RO_COMPAT_LARGE_FILE| \
EXT4_FEATURE_RO_COMPAT_DIR_NLINK)
-#define FEATURE_INCOMPAT_IGNORE (EXT3_FEATURE_INCOMPAT_EXTENTS)
+#define FEATURE_INCOMPAT_IGNORE (EXT3_FEATURE_INCOMPAT_EXTENTS| \
+ EXT3_FEATURE_INCOMPAT_RECOVER)

int check_backup_super_block(e2fsck_t ctx)
{



2008-11-22 17:37:01

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] e2fsck: ignore differing NEEDS_RECOVERY flag on backup sbs

On Nov 22, 2008 09:02 -0600, Eric Sandeen wrote:
> This is for RH bugzilla 471925 - Complete scan of filesystems expanded online
>
> When we resize online, the primary superblock gets copied to all
> the backups, and of course since we're mounted the NEEDS_RECOVERY
> flag is set. A subsequent fsck will find the backups have the
> NEEDS_RECOVERY flag set while the primary does not, and this
> forces a full fsck pass.
>
> I think this flag can be safely ignored in the flag comparisons.

Should we also mask out this flag from the backup superblock copies
when they are made, or is there an equal chance that the superblock
has NEEDS_RECOVERY and the backups do not?

> Signed-off-by: Eric Sandeen <[email protected]>
> ---
>
> Index: e2fsprogs/e2fsck/super.c
> ===================================================================
> --- e2fsprogs.orig/e2fsck/super.c 2008-11-22 07:54:47.000000000 -0600
> +++ e2fsprogs/e2fsck/super.c 2008-11-22 08:59:45.953060973 -0600
> @@ -860,7 +860,8 @@ void check_super_block(e2fsck_t ctx)
> * try to discourage it in the future. In particular, for the newer
> * ext4 files, especially EXT4_FEATURE_RO_COMPAT_DIR_NLINK and
> * EXT3_FEATURE_INCOMPAT_EXTENTS. So some of these may go away in the
> - * future.
> + * future. EXT3_FEATURE_INCOMPAT_RECOVER may also get set when
> + * copying the primary superblock during online resize.
> *
> * The kernel will set EXT2_FEATURE_COMPAT_EXT_ATTR, but
> * unfortunately, we shouldn't ignore it since if it's not set in the
> @@ -869,7 +870,8 @@ void check_super_block(e2fsck_t ctx)
> */
> #define FEATURE_RO_COMPAT_IGNORE (EXT2_FEATURE_RO_COMPAT_LARGE_FILE| \
> EXT4_FEATURE_RO_COMPAT_DIR_NLINK)
> -#define FEATURE_INCOMPAT_IGNORE (EXT3_FEATURE_INCOMPAT_EXTENTS)
> +#define FEATURE_INCOMPAT_IGNORE (EXT3_FEATURE_INCOMPAT_EXTENTS| \
> + EXT3_FEATURE_INCOMPAT_RECOVER)
>
> int check_backup_super_block(e2fsck_t ctx)
> {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.


2008-11-23 03:29:11

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] e2fsck: ignore differing NEEDS_RECOVERY flag on backup sbs

Andreas Dilger wrote:
> On Nov 22, 2008 09:02 -0600, Eric Sandeen wrote:
>> This is for RH bugzilla 471925 - Complete scan of filesystems expanded online
>>
>> When we resize online, the primary superblock gets copied to all
>> the backups, and of course since we're mounted the NEEDS_RECOVERY
>> flag is set. A subsequent fsck will find the backups have the
>> NEEDS_RECOVERY flag set while the primary does not, and this
>> forces a full fsck pass.
>>
>> I think this flag can be safely ignored in the flag comparisons.
>
> Should we also mask out this flag from the backup superblock copies
> when they are made, or is there an equal chance that the superblock
> has NEEDS_RECOVERY and the backups do not?

I think it might be a good idea (to mask at growfs time) for
completeness. Is there *ever* any valid reason for a backup superblock
to have this flag set? Near as I can tell, online growfs is the only
thing that ever sets it, and this "flag match" check is the only thing
that ever tests it.

-Eric

2009-03-26 16:31:45

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] e2fsck: ignore differing NEEDS_RECOVERY flag on backup sbs

Eric Sandeen wrote:
> This is for RH bugzilla 471925 - Complete scan of filesystems expanded online
>
> When we resize online, the primary superblock gets copied to all
> the backups, and of course since we're mounted the NEEDS_RECOVERY
> flag is set. A subsequent fsck will find the backups have the
> NEEDS_RECOVERY flag set while the primary does not, and this
> forces a full fsck pass.
>
> I think this flag can be safely ignored in the flag comparisons.

ping on this? Andreas wondered about masking the flag when sb backups
are written, but I think that only needs to be done in kernelspace; from
reading code & testing offline resize, I don't think userspace needs
further changes.

-Eric

> Signed-off-by: Eric Sandeen <[email protected]>
> ---
>
> Index: e2fsprogs/e2fsck/super.c
> ===================================================================
> --- e2fsprogs.orig/e2fsck/super.c 2008-11-22 07:54:47.000000000 -0600
> +++ e2fsprogs/e2fsck/super.c 2008-11-22 08:59:45.953060973 -0600
> @@ -860,7 +860,8 @@ void check_super_block(e2fsck_t ctx)
> * try to discourage it in the future. In particular, for the newer
> * ext4 files, especially EXT4_FEATURE_RO_COMPAT_DIR_NLINK and
> * EXT3_FEATURE_INCOMPAT_EXTENTS. So some of these may go away in the
> - * future.
> + * future. EXT3_FEATURE_INCOMPAT_RECOVER may also get set when
> + * copying the primary superblock during online resize.
> *
> * The kernel will set EXT2_FEATURE_COMPAT_EXT_ATTR, but
> * unfortunately, we shouldn't ignore it since if it's not set in the
> @@ -869,7 +870,8 @@ void check_super_block(e2fsck_t ctx)
> */
> #define FEATURE_RO_COMPAT_IGNORE (EXT2_FEATURE_RO_COMPAT_LARGE_FILE| \
> EXT4_FEATURE_RO_COMPAT_DIR_NLINK)
> -#define FEATURE_INCOMPAT_IGNORE (EXT3_FEATURE_INCOMPAT_EXTENTS)
> +#define FEATURE_INCOMPAT_IGNORE (EXT3_FEATURE_INCOMPAT_EXTENTS| \
> + EXT3_FEATURE_INCOMPAT_RECOVER)
>
> int check_backup_super_block(e2fsck_t ctx)
> {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2009-04-07 17:22:16

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] e2fsck: ignore differing NEEDS_RECOVERY flag on backup sbs

On Sat, Nov 22, 2008 at 09:02:48AM -0600, Eric Sandeen wrote:
> This is for RH bugzilla 471925 - Complete scan of filesystems expanded online

Applied to the maint branch.

- Ted