Hi Eric,
I've seen you guys had some open RH bugs on ext3, who all share in
common the "bit already free" error.
This bug I reported can explain many different problems in ext[34].
Essentially, every time there is a kernel crash (or hard reboot)
during delete/truncate of a large file,
it may result in "bit already clear" error after reboot.
The problem is very simple and so is the fix.
I proved the problem with 100% recreation chances using a small patch,
instead of running statistical stress tests.
All I did was to add a print and 10 seconds delay after transaction
restart in ext3_free_branches and reboot > 5 seconds after the
transaction restarts, so that kjournald will have time to commit the
old transaction.
After the reboot, I always get "bit already clear" errors, because the
"half large truncate" transaction is not handled properly.
I did not get any response from ext4 guys so far and since this bug
dates back to ext3,
I was hoping you guys could take a look and put your weight on pushing
the fix upstream.
Thanks,
Amir.
On Wed, Jun 23, 2010 at 9:27 PM, Amir G. <amir73il@xxxxxxxxxxxxxxxxxxxxx> wrote:
> Sorry, posting the patch again with a better title.
> This bug needs to be fixed also in Ext3.
> just need to move the ext3_forget() line down to ext3_free_blocks() line.
>
> Amir.
>
> On Wed, Jun 23, 2010 at 10:01 PM, Amir G.
> <amir73il@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> Hi,
>>
>> We have experienced bitmap inconsistencies after crash during file
>> delete under heavy load.
>> The crash is not file system related and I the following patch in
>> ext4_free_branches() fixes the recovery problem.
>>
>> If the transaction is restarted and there is a crash before the new
>> transaction is committed,
>> then after recovery, the blocks that this indirect block points to
>> have been freed, but the indirect block itself
>> has not been freed and may still point to some of the free blocks
>> (because of the ext4_forget()).
>>
>> So ext4_forget() should be called inside ext4_free_blocks() to avoid
>> this problem.
>> Are there any consequences to this patch that I am not aware of?
>>
>> Amir.
>>
>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxxxxx>
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 42272d6..682e2fa 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4458,6 +4458,7 @@ static void ext4_free_branches(handle_t *handle,
>> struct inode *inode,
>> {
>> ext4_fsblk_t nr;
>> __le32 *p;
>> + int flags;
>>
>> if (ext4_handle_is_aborted(handle))
>> return;
>> @@ -4520,7 +4521,7 @@ static void ext4_free_branches(handle_t *handle,
>> struct inode *inode,
>> * revoke records must be emitted *before* clearing
>> * this block's bit in the bitmaps.
>> */
>> - ext4_forget(handle, 1, inode, bh, bh->b_blocknr);
>> + flags =
>> EXT4_FREE_BLOCKS_METADATA|EXT4_FREE_BLOCKS_FORGET;
>>
>> /*
>> * Everything below this this pointer has been
>> @@ -4546,8 +4547,7 @@ static void ext4_free_branches(handle_t *handle,
>> struct inode *inode,
>> blocks_for_truncate(inode));
>> }
>>
>> - ext4_free_blocks(handle, inode, 0, nr, 1,
>> - EXT4_FREE_BLOCKS_METADATA);
>> + ext4_free_blocks(handle, inode, 0, nr, 1, flags);
>>
>> if (parent_bh) {
>> /*
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
