Re: [PATCH v2] ext4: record writeback error when discarding pages on failed delalloc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ext4: folio reference leak on error path in mpage_submit_partial_folio()
--

commit 828f6b6c9cdd25b981807944a30cd22b2612d38e
Author: guolingxing <guolingxing@tju.edu.cn>
Subject: ext4: record writeback error when discarding pages on failed delalloc

This commit records writeback errors when block mapping fails and dirty
folios are discarded. It also scopes the give_up_on_write flag to each
writeback loop iteration to prevent transient errors from invalidating
dirty folios after a fatal ENOSPC retry.

> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 6ede3644a738b..b3b9650b32116 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2637,6 +2637,12 @@ static int mpage_map_and_submit_extent(handle_t *handle,
>  				ext4_print_free_blocks(inode);
>  		invalidate_dirty_pages:
>  			*give_up_on_write = true;
> +			/*
> +			 * Record the error so that a subsequent fsync()
> +			 * / errseq check can observe the data loss that
> +			 * discarding these dirty pages implies.
> +			 */
> +			mapping_set_error(inode->i_mapping, err);

[Severity: Medium]
Is there a memory leak in the error path of mpage_submit_partial_folio()?

When called from mpage_map_and_submit_extent() and jumping to
invalidate_dirty_pages:

fs/ext4/inode.c:mpage_map_and_submit_extent() {
    ...
				if (progress) {
					if (mpage_submit_partial_folio(mpd))
						goto invalidate_dirty_pages;
    ...
}

The call to mpage_submit_partial_folio() can return an error without
releasing the folio reference:

fs/ext4/inode.c:mpage_submit_partial_folio() {
    ...
	folio = filemap_get_folio(inode->i_mapping,
				  mpd->start_pos >> PAGE_SHIFT);
	if (IS_ERR(folio))
		return PTR_ERR(folio);
    ...
	pos = ((loff_t)mpd->map.m_lblk) << inode->i_blkbits;
	if (WARN_ON_ONCE((folio_pos(folio) == pos) ||
			 !folio_contains(folio, pos >> PAGE_SHIFT)))
		return -EINVAL;
    ...
}

If the WARN_ON_ONCE bounds check fails, it returns -EINVAL early without
calling folio_put(folio). Can this prevent the folio from being freed,
potentially causing hangs during unmount, inode eviction, or memory
reclaim?

>  			return err;
>  		}
>  		progress = 1;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016110311.71391-1-guolingxing@tju.edu.cn?part=1