Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Date:   Wed, 27 Sep 2023 14:43:24 +0530
Message-Id: <87ttrgf07f.fsf@doe.com>
From:   Ritesh Harjani (IBM) <ritesh.list@gmail.com>
To:     Kemeng Shi <shikemeng@huaweicloud.com>, tytso@mit.edu,
        adilger.kernel@dilger.ca
Cc:     ojaswin@linux.ibm.com, linux-ext4@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 04/12] ext4: extend ext4_mb_mark_context to support allocation under journal
In-Reply-To: <20230919201532.310085-5-shikemeng@huaweicloud.com>
Precedence: bulk

Kemeng Shi <shikemeng@huaweicloud.com> writes:

> Previously, ext4_mb_mark_context is only called under fast commit
> replay path, so there is no valid handle when we update block bitmap
> and group descriptor. This patch try to extend ext4_mb_mark_context
> to be used by code under journal. There are several improvement:
> 1. add "handle_t *handle" to struct ext4_mark_context to journal block
> bitmap and group descriptor update inside ext4_mb_mark_context (the
> added journal code is based on ext4_mb_mark_diskspace_used where
> ext4_mb_mark_context is going to be used.)

> 2. add EXT4_MB_BITMAP_MARKED_CHECK flag to control check if bits in block
> bitmap are already marked as allocation code under journal asserts that
> all bits to be changed are not marked before.

Maybe we can reword this to... 
Adds a flag argument to ext4_mb_mark_context() which controls
a. EXT4_MB_BITMAP_MARKED_CHECK - whether block bitmap checking is needed.
b. EXT4_MB_SYNC_UPDATE - whether dirty buffers (bitmap and group descriptor) needs sync.


> 3. add "ext4_grpblk_t changed" to struct ext4_mark_context to notify number
> of bits in block bitmap has changed.

We should remove above point 3 as there is no "struct ext4_mark_context"
in this v7 series.

>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  fs/ext4/mballoc.c | 64 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 49 insertions(+), 15 deletions(-)


The changes looks good to me. With commit msg updated, feel free to add-

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