On Thu 10-10-24 10:58:55, Baolin Liu wrote:
> From: Baolin Liu <liubaolin@xxxxxxxxxx>
>
> Since the merge of commit 3910b513fcdf ("ext4: persist the new uptodate
> buffers in ext4_journalled_zero_new_buffers"), a new assertion failure
> occurred under a old kernel(ext3, data=journal, pagesize=64k) with
> corresponding ported patches:
...
> which was caused by bh dirting without calling
> do_journal_get_write_access().
>
> In the loop for all bhs of a page in ext4_block_write_begin(),
> when a err occurred, it will jump out of loop.
> But that will leaves some bhs being processed and some not,
> which will lead to the asserion failure in calling write_end_fn().
Thanks for the patch but I don't understand one thing here: For
ext4_journalled_zero_new_buffers() to call write_end_fn() the buffer must
have buffer_new flag set. That flag can get set only by ext4_get_block()
function when it succeeds in which case we also call
do_journal_get_write_access(). So how is it possible that buffer_new was
set on a buffer on which we didn't call do_journal_get_write_access()? This
indicates there may be some deeper problem hidden. How exactly did you
trigger this problem?
Honza
>
> To fixed that, get write access for the rest unprocessed bhs, just
> as what write_end_fn do.
>
> Fixes: 3910b513fcdf ("ext4: persist the new uptodate buffers in ext4_journalled_zero_new_buffers")
> Reported-and-tested-by: Zhi Long <longzhi@xxxxxxxxxxxxxx>
> Suggested-by: Shida Zhang <zhangshida@xxxxxxxxxx>
> Signed-off-by: Baolin Liu <liubaolin@xxxxxxxxxx>
> ---
> fs/ext4/inode.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 54bdd4884fe6..a72f951288e4 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1102,9 +1102,24 @@ int ext4_block_write_begin(handle_t *handle, struct folio *folio,
> err = -EIO;
> }
> if (unlikely(err)) {
> - if (should_journal_data)
> + if (should_journal_data) {
> + if (bh != head || !block_start) {
> + do {
> + block_end = block_start + bh->b_size;
> +
> + if (buffer_new(bh))
> + if (block_end > from && block_start < to)
> + do_journal_get_write_access(handle,
> + inode, bh);
> +
> + block_start = block_end;
> + bh = bh->b_this_page;
> + } while (bh != head);
> + }
> +
> ext4_journalled_zero_new_buffers(handle, inode, folio,
> from, to);
> + }
> else
> folio_zero_new_buffers(folio, from, to);
> } else if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
> --
> 2.39.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
