Re: [Ext2-devel] Re: ext3: bump mount count on journal replay

From: Pavel Machek
Date: Fri Jul 16 2004 - 16:09:15 EST


Hi!

> > > AFAICS, this just means that if you have an ext3 filesystem
> > > (i.e. has_journal) that you will fsck 5x as often, not so great. You
> > > should instead check for INCOMPAT_RECOVER instead of HAS_JOURNAL.
> >
> > Oops, you are right. Updated patch is attached.
>
> No patch was attached.

Sorry, here it is:

--- clean/fs/ext3/super.c 2004-06-22 12:36:30.000000000 +0200
+++ linux/fs/ext3/super.c 2004-07-14 22:32:20.000000000 +0200
@@ -919,7 +919,7 @@
}

static int ext3_setup_super(struct super_block *sb, struct ext3_super_block *es,
- int read_only)
+ int read_only, int mount_cost)
{
struct ext3_sb_info *sbi = EXT3_SB(sb);
int res = 0;
@@ -960,7 +960,7 @@
if (!(__s16) le16_to_cpu(es->s_max_mnt_count))
es->s_max_mnt_count =
(__s16) cpu_to_le16(EXT3_DFL_MAX_MNT_COUNT);
- es->s_mnt_count=cpu_to_le16(le16_to_cpu(es->s_mnt_count) + 1);
+ es->s_mnt_count=cpu_to_le16(le16_to_cpu(es->s_mnt_count) + mount_cost);
es->s_mtime = cpu_to_le32(get_seconds());
ext3_update_dynamic_rev(sb);
EXT3_SET_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER);
@@ -1214,7 +1214,7 @@
int hblock;
int db_count;
int i;
- int needs_recovery;
+ int needs_recovery, mount_cost = 1;

sbi = kmalloc(sizeof(*sbi), GFP_KERNEL);
if (!sbi)
@@ -1478,6 +1478,8 @@
needs_recovery = (es->s_last_orphan != 0 ||
EXT3_HAS_INCOMPAT_FEATURE(sb,
EXT3_FEATURE_INCOMPAT_RECOVER));
+ if (needs_recovery)
+ mount_cost = 5;

/*
* The first inode we look at is the journal inode. Don't try
@@ -1485,8 +1487,8 @@
*/
if (!test_opt(sb, NOLOAD) &&
EXT3_HAS_COMPAT_FEATURE(sb, EXT3_FEATURE_COMPAT_HAS_JOURNAL)) {
- if (ext3_load_journal(sb, es))
- goto failed_mount2;
+ if (ext3_load_journal(sb, es))
+ goto failed_mount2;
} else if (journal_inum) {
if (ext3_create_journal(sb, es, journal_inum))
goto failed_mount2;
@@ -1543,7 +1545,7 @@
goto failed_mount3;
}

- ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
+ ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY, mount_cost);
/*
* akpm: core read_super() calls in here with the superblock locked.
* That deadlocks, because orphan cleanup needs to lock the superblock
@@ -2069,7 +2071,7 @@
*/
ext3_clear_journal_err(sb, es);
sbi->s_mount_state = le16_to_cpu(es->s_state);
- if (!ext3_setup_super (sb, es, 0))
+ if (!ext3_setup_super (sb, es, 0, 1))
sb->s_flags &= ~MS_RDONLY;
}
}


> > > Instead, you could change this to only increment the mount count after
> > > a clean unmount 20% of the time (randomly). Since most people bitch
> > > about the full fsck anyways this is probably the better choice than
> > > increasing the frequency of checks and forcing the users to change the
> > > check interval to get the old behaviour.
> >
> > Nice hack.... would that be acceptable?
>
> It's OK by me. I don't think you'll get complaints from users if it is
> checked less often (there is still the time-based check).

Hmmm... I guess that using get_random_bytes is pretty easy. Completely
untested diff (have to sleep now):

--- clean/fs/ext3/super.c 2004-06-22 12:36:30.000000000 +0200
+++ linux/fs/ext3/super.c 2004-07-16 23:05:30.000000000 +0200
@@ -919,7 +919,7 @@
}

static int ext3_setup_super(struct super_block *sb, struct ext3_super_block *es,
- int read_only)
+ int read_only, int mount_cost)
{
struct ext3_sb_info *sbi = EXT3_SB(sb);
int res = 0;
@@ -960,7 +960,7 @@
if (!(__s16) le16_to_cpu(es->s_max_mnt_count))
es->s_max_mnt_count =
(__s16) cpu_to_le16(EXT3_DFL_MAX_MNT_COUNT);
- es->s_mnt_count=cpu_to_le16(le16_to_cpu(es->s_mnt_count) + 1);
+ es->s_mnt_count=cpu_to_le16(le16_to_cpu(es->s_mnt_count) + mount_cost);
es->s_mtime = cpu_to_le32(get_seconds());
ext3_update_dynamic_rev(sb);
EXT3_SET_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER);
@@ -1214,7 +1214,11 @@
int hblock;
int db_count;
int i;
- int needs_recovery;
+ int needs_recovery, mount_cost;
+ unsigned char random;
+
+ get_random_bytes(&random, 1);
+ mount_cost = (random < 60);

sbi = kmalloc(sizeof(*sbi), GFP_KERNEL);
if (!sbi)
@@ -1478,6 +1482,8 @@
needs_recovery = (es->s_last_orphan != 0 ||
EXT3_HAS_INCOMPAT_FEATURE(sb,
EXT3_FEATURE_INCOMPAT_RECOVER));
+ if (needs_recovery)
+ mount_cost = 1;

/*
* The first inode we look at is the journal inode. Don't try
@@ -1485,8 +1491,8 @@
*/
if (!test_opt(sb, NOLOAD) &&
EXT3_HAS_COMPAT_FEATURE(sb, EXT3_FEATURE_COMPAT_HAS_JOURNAL)) {
- if (ext3_load_journal(sb, es))
- goto failed_mount2;
+ if (ext3_load_journal(sb, es))
+ goto failed_mount2;
} else if (journal_inum) {
if (ext3_create_journal(sb, es, journal_inum))
goto failed_mount2;
@@ -1543,7 +1549,7 @@
goto failed_mount3;
}

- ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
+ ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY, mount_cost);
/*
* akpm: core read_super() calls in here with the superblock locked.
* That deadlocks, because orphan cleanup needs to lock the superblock
@@ -2069,7 +2075,7 @@
*/
ext3_clear_journal_err(sb, es);
sbi->s_mount_state = le16_to_cpu(es->s_state);
- if (!ext3_setup_super (sb, es, 0))
+ if (!ext3_setup_super (sb, es, 0, 1))
sb->s_flags &= ~MS_RDONLY;
}
}

Pavel



--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/