Re: [PATCH 3/3] ext4: check return value of blkdev_issue_flush() when replaying fast commits

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ext4: regression causing mount failures on devices lacking flush support
- [Medium] ext4: severe mount delay due to O(N) per-inode fast commit cache flush
--

commit 1d9ac3ed6ced15861bdc5e6964915fbce6a1b1e6
Author: guolingxing <guolingxing@tju.edu.cn>

ext4: check return value of blkdev_issue_flush() when replaying fast commits

This patch updates ext4_fc_replay_inode() to check and return the error
code from blkdev_issue_flush(). It intends to abort the fast commit replay
and fail the mount if the cache flush fails.

Cc: stable@vger.kernel.org

> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index ead8b16f9be07..f554405ebabfe 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -2019,7 +2019,7 @@ static int ext4_fc_replay_inode(struct super_block *sb,
>  out:
>  	iput(inode);
>  	if (!ret)
> -		blkdev_issue_flush(sb->s_bdev);
> +		ret = blkdev_issue_flush(sb->s_bdev);

[Severity: High]
Does this code introduce a regression for devices that do not support cache
flushes?

If the underlying block device (such as certain loop devices, ramdisks,
or older flash media) does not support flushes, blkdev_issue_flush() will
return -EOPNOTSUPP. Ext4 usually ignores -EOPNOTSUPP for flushes, but this
change unconditionally captures and returns the error code to the fast
commit replay mechanism.

As a result, fast commit replay will abort and the entire filesystem mount
will fail on these devices.

[Severity: Medium]
Can this flush cause severe mount delays during fast commit recovery?

Because ext4_fc_replay_inode() is called for every inode in the fast commit
journal, this will trigger an O(N) number of full device cache flushes.
This can cause significant performance degradation when processing a journal
with many EXT4_FC_TAG_INODE tags.

Additionally, is this flush structurally redundant? JBD2 already guarantees
a final blkdev_issue_flush() on the filesystem device (journal->j_fs_dev)
in jbd2_journal_recover() after all recovery passes (including fast commit
replay) are complete:

fs/jbd2/recovery.c:jbd2_journal_recover() {
    ...
    /* Make sure all replayed data is on permanent storage */
    if (journal->j_flags & JBD2_BARRIER) {
        err2 = blkdev_issue_flush(journal->j_fs_dev);
        if (!err)
            err = err2;
    }
    ...
}

>  
>  	return ret;
>  }

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