Re: [PATCH v7 14/31] ext4: rework handle credit accounting for unwritten extent conversion

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ext4: atomicity violation in fallocate extent conversion
--

commit 352cf8eb3125c707efc06205d4a61cc670cd67bf
Author: Zhang Yi <yi.zhang@huawei.com>

ext4: rework handle credit accounting for unwritten extent conversion

This commit updates the unwritten extent conversion process to handle long
merged ioends. It introduces a check using ext4_journal_ensure_credits to
extend or restart the transaction if credits run low during the loop.

> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 76038b6c36552..83d040e36b682 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
[ ... ]
> @@ -5137,27 +5137,36 @@ int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
[ ... ]
>  	while (max_blocks) {
> -		if (credits) {
> -			handle = ext4_journal_start(inode, EXT4_HT_MAP_BLOCKS,
> -						    credits);
> -			if (IS_ERR(handle)) {
> -				ret = PTR_ERR(handle);
> -				break;
> -			}
> -		}
> +		/*
> +		 * The caller cannot know how many extents the range covers,
> +		 * so make sure the transaction can take one more extent,
> +		 * extending or restarting it once it is full.  The reserved
> +		 * handle of the buffer_head writeback path is safe as it has
> +		 * enough credits for the conversion, so it is not restarted
> +		 * here.
> +		 */
> +		ret = ext4_journal_ensure_credits(handle, credits, 0);
> +		if (ret < 0)
> +			break;

[Severity: High]
Does restarting the caller's transaction here break atomicity requirements
for fallocate?

ext4_alloc_file_blocks relies on the atomicity of converting unwritten
extents to written and adding the inode to the orphan list. This prevents
e2fsck from flagging initialized extents beyond i_disksize as corruption on
crash.

If the transaction restarts during this conversion loop, the first batch of
converted extents is committed to the journal before the loop finishes and
ext4_orphan_add is called in the parent function:

fs/ext4/extents.c:ext4_alloc_file_blocks() {
    ...
    ret = ext4_convert_unwritten_extents(handle, inode, ...);
    ...
    if (new_size && converted) {
        ret2 = ext4_orphan_add(handle, inode);
    ...
}

If a crash occurs after the transaction restarts but before ext4_orphan_add
completes, will it leave the disk with initialized extents beyond i_disksize
and no orphan list entry, triggering filesystem corruption warnings and
silent truncations in e2fsck?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009103033.2920530-1-yi.zhang@huaweicloud.com?part=14