linux-ext4 - Re: fsck errors encountered when applying patch "ext4: fix BUG when calling ext4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 20 Feb 2009 20:29:08 -0500
From:	Theodore Tso <tytso@....edu>
To:	Xiang Wang <xiangw@...gle.com>
Cc:	linux-ext4@...r.kernel.org, aneesh.kumar@...ux.vnet.ibm.com,
	Michael Rubin <mrubin@...gle.com>,
	Curt Wohlgemuth <curtw@...gle.com>,
	Chad Talbott <ctalbott@...gle.com>,
	Frank Mayhar <fmayhar@...gle.com>
Subject: Re: fsck errors encountered when applying patch "ext4: fix BUG
	when calling ext4_error with locked block group"

On Fri, Feb 20, 2009 at 03:31:36PM -0800, Xiang Wang wrote:
> We hit filesystem errors reported by fsck after we run dbench, an
> example of the error is as follows:
> // run dbench
> dbench complete!
> starting fsck...
> e2fsck 1.41.3 (12-Oct-2008)
> /dev/hdk3 contains a file system with errors, check forced.

Hmm, that's interesting.  Can you check the system logs and see what
sort of errors were flagged by ext4?  There may have been some sort of
issue which caused the filesystem to set the ERROR_FS flag.  It would
be useful to see what the filesystem complained about.

> This problem seems to be the number of free inodes stored in the ext4
> super block does not match the number counted by reading the inode
> bitmaps.
> 
> Then I looked into the patch, especially the diff in the
> ext4_commit_super in fs/ext4/super.c
> http://git.kernel.org/?p=linux/kernel/git/tytso/ext4.git;a=blobdiff;f=fs/ext4/super.c;h=c53cab1e0a7fca1d9406f9bbb7c9cb661bae0567;hp=ed0406de6cae7379df9f72f272ade1a18df3966b;hb=be8f3df12cddeb352dd624fba9bf46a2de5711f3;hpb=e8470671cf71ec6361b71b3c95a1a1392c5cfa75
> 
> @@ -2868,8 +2906,11 @@ static void ext4_commit_super(struct super_block *sb,
>                 set_buffer_uptodate(sbh);
>         }
>         es->s_wtime = cpu_to_le32(get_seconds());
> -       ext4_free_blocks_count_set(es, ext4_count_free_blocks(sb));
> -       es->s_free_inodes_count = cpu_to_le32(ext4_count_free_inodes(sb));
> +       ext4_free_blocks_count_set(es, percpu_counter_sum_positive(
> +                                       &EXT4_SB(sb)->s_freeblocks_counter));
> +       es->s_free_inodes_count = cpu_to_le32(percpu_counter_sum_positive(
> +                                       &EXT4_SB(sb)->s_freeinodes_counter));
> +
>         BUFFER_TRACE(sbh, "marking dirty");
>         mark_buffer_dirty(sbh);
>         if (sync) {
> 
> seems like the new code only looks into the s_freeinodes_counter field
> while the old code calls ext4_count_free_inodes(sb) and calculates the
> count by adding up the free inode number from each block group.

I'm guessing the problem here is that you didn't take these commits:

	02d211688727ad02bb4555b1aa8ae2de16b21b39
	71c5576fbd809f2015f4eddf72e501e298720cf3

... which fixes a problem which would cause inaccuracies in the percpu
counters due to races that would be tend to show up under a workload
such as dbench.  Read the commit description of these two commits very
carefully, since there are potentially other users of the percpu
counters, and this changes the semantics of percpu_counter_sum().

Note that the percpu_counter race conditions can lead to problems
where the delayed allocation code might not notice that the disk has
filled, leading to data being lost without the application getting an
ENOSPC return from a write() system call.

One of the downsides of trying to backport a lot of changes to a
kernel version as old as 2.6.26 is that you miss changes like this.
You might want to think about leapfrogging to a newer kernel at some
point....

						- Ted

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