Re: [PATCH v7 05/12] ext4: call ext4_mb_mark_context in ext4_mb_mark_diskspace_used

From: IBM
Date: Wed Sep 27 2023 - 05:58:48 EST


Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx> writes:

> call ext4_mb_mark_context in ext4_mb_mark_diskspace_used to:
> 1. remove repeat code to normally update bitmap and group descriptor
> on disk.
> 2. call ext4_mb_mark_context instead of only setting bits in block bitmap
> to fix the bitmap. Function ext4_mb_mark_context will also update
> checksum of bitmap and other counter along with the bit change to keep
> the cosistent with bit change or block bitmap will be marked corrupted as
> checksum of bitmap is in inconsistent state.
>

Rewording point 2 to...
Now that we have a common API for marking blocks inuse/free in block
bitmap, use that instead of open coding it in function
ext4_mb_mark_diskspace_used(). The current code was not updating
checksum and other counters. ext4_mb_mark_context() should fix these
consistency problems.


Also I now see why you have used "int" (ext4_grpblk_t) for len in
ext4_mb_mark_context(). The reason is because this is "cluster len" which
is defined as ext4_grpblk_t in "struct ext4_free_extent"

I think by default we anyway have 8192 blocks per block group. So it
should be ok for now. I anyway think the usage of blocks and cluster (&
their data type) is confusing at different places which needs an auditing/cleanup.

This patch looks good to me. Feel free to add -

Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>