Re: [PATCH] ext4: Automatic setting of {INODE,BLOCK}_UNINIT flags

[Theodore Ts'o <tytso@mit.edu>]

On Thu, Oct 25, 2012 at 09:25:43AM +0200, Tomas Racek wrote:
> When last inode from bg is freed, set the INODE_UNINIT flag, similarly
> when last block is freed, set BLOCK_UNINIT flag. This can speed up
> subsequent fsck run.
> 
> Signed-off-by: Tomas Racek <tracek@redhat.com>

Could you make the following enhancements to your patch?

1)  Only do this if ext4_has_group_desc_csum(sb) is true

2)  Check to make sure the inode bitmap has only one bit set (the inode
    to be freed).  Basically, I don't want to set the BLOCK/INODE_UNINIT
    based just on the block group descriptor count, in case it got corrupted
     --- what, me paranoid?

    If there is other inodes set, then we need to call ext4_error()
    since we know the file system has gotten corrupted.

3)  If we can set BLOCK/INODE_UNINIT, we can skip modifying the bitmap
    block (since we won't consult the bitmap block if uninit is set).
    So that means we can skip calling get_write_access(), modifying
    the bitmap, updating the checksum, and then calling
    handle_dirty_metadata.  In fact, we can call ext4_forget(), so we
    can drop it from the buffer cache, and if it had been modified during
    the current transaction --- as part of an rm -rf, for example ---
    we don't need to include the bitmap block in the transaction.

(2) makes this patch much safer, and (3) should more than make up for
the overhead of scanning the the bitmap.

Thanks!!!

						- Ted

P.S.  Although the block bitmap is metadata, you can pass 0 for
is_metadata, since we don't need to revoke the block --- that's only
needed for extent tree blocks, indirect blocks, or directory blocks,
which could potentially get reused as a data block.  This isn't an
issue for the bitmap blocks, so we don't need to include a revoke
record in the journal.