Messages in this thread Patch in this message |  | | From | Daniel Phillips <> | Subject | [PATCH] Re: 2.4.0-test11 ext2 fs corruption | Date | Thu, 30 Nov 2000 01:03:37 +0100 |
| |
Alexander Viro wrote: > Bloody hell...
I don't know if this is the bug he's got, in fact I doubt it, but it's a bug and it needs fixing. The problem is, ext2_get_group_desc effectively returns two results; one of them is being assigned from on conditional paths and the other isn't. This bug will cause - on very rare occasions - the wrong group descriptor block to be marked dirty, and changes might be lost. I think what we'd see as a result is wrong block, inode and directory counts.
The fix below is kind of gross. The way I really want to do the fix is to remove one parameter from ext2_get_group_desc and thereby get rid of the troublesome side effect for good, but that kind of change isn't compatible with 'code freeze'.
(linux.2.4.0-test11)
--- fs/ext2/ialloc.c.old Thu Nov 30 00:36:02 2000 +++ fs/ext2/ialloc.c Thu Nov 30 00:36:39 2000 @@ -260,7 +260,7 @@ { struct super_block * sb; struct buffer_head * bh; - struct buffer_head * bh2; + struct buffer_head * bh2, * tmpbh2; int i, j, avefreei; struct inode * inode; int bitmap_nr; @@ -293,10 +293,11 @@ /* I am not yet convinced that this next bit is necessary. i = dir->u.ext2_i.i_block_group; for (j = 0; j < sb->u.ext2_sb.s_groups_count; j++) { - tmp = ext2_get_group_desc (sb, i, &bh2); + tmp = ext2_get_group_desc (sb, i, &tmpbh2); if (tmp && (le16_to_cpu(tmp->bg_used_dirs_count) << 8) < le16_to_cpu(tmp->bg_free_inodes_count)) { + bh2 = tmpbh2; gdp = tmp; break; } @@ -306,7 +307,7 @@ */ if (!gdp) { for (j = 0; j < sb->u.ext2_sb.s_groups_count; j++) { - tmp = ext2_get_group_desc (sb, j, &bh2); + tmp = ext2_get_group_desc (sb, j, &tmpbh2); if (tmp && le16_to_cpu(tmp->bg_free_inodes_count) && le16_to_cpu(tmp->bg_free_inodes_count) >= avefreei) { @@ -314,6 +315,7 @@ (le16_to_cpu(tmp->bg_free_blocks_count) > le16_to_cpu(gdp->bg_free_blocks_count))) { i = j; + bh2 = tmpbh2; gdp = tmp; } } @@ -326,11 +328,11 @@ * Try to place the inode in its parent directory */ i = dir->u.ext2_i.i_block_group; - tmp = ext2_get_group_desc (sb, i, &bh2); - if (tmp && le16_to_cpu(tmp->bg_free_inodes_count)) + tmp = ext2_get_group_desc (sb, i, &tmpbh2); + if (tmp && le16_to_cpu(tmp->bg_free_inodes_count)) { + bh2 = tmpbh2; gdp = tmp; - else - { + } else { /* * Use a quadratic hash to find a group with a * free inode @@ -339,9 +341,10 @@ i += j; if (i >= sb->u.ext2_sb.s_groups_count) i -= sb->u.ext2_sb.s_groups_count; - tmp = ext2_get_group_desc (sb, i, &bh2); + tmp = ext2_get_group_desc (sb, i, &tmpbh2); if (tmp && le16_to_cpu(tmp->bg_free_inodes_count)) { + bh2 = tmpbh2; gdp = tmp; break; } @@ -355,9 +358,10 @@ for (j = 2; j < sb->u.ext2_sb.s_groups_count; j++) { if (++i >= sb->u.ext2_sb.s_groups_count) i = 0; - tmp = ext2_get_group_desc (sb, i, &bh2); + tmp = ext2_get_group_desc (sb, i, &tmpbh2); if (tmp && le16_to_cpu(tmp->bg_free_inodes_count)) { + bh2 = tmpbh2; gdp = tmp; break; } -- Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
|  |