Re: [RFC PATCH v2 2/3] btrfs: Allow error injection on btrfs_data_csum_ok

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The error injection completely bypasses the error reporting and notification logic it is intended to test.
--

--- Patch [2]: [RFC PATCH v2 2/3] btrfs: Allow error injection on btrfs_data_csum_ok ---
commit 1a83f56a9eedee07c1baca182513e1a6c13386dd
Author: Song Chen <chensong_2000@126.com>

btrfs: Allow error injection on btrfs_data_csum_ok

This patch marks btrfs_data_csum_ok with ALLOW_ERROR_INJECTION to allow the
fail_function infrastructure to simulate checksum mismatches. The intent is
to validate error detection and reporting paths in a controlled environment.

> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
[ ... ]
> @@ -3397,6 +3397,7 @@ bool btrfs_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
>  		memzero_page(phys_to_page(paddrs[i]), offset_in_page(paddrs[i]), step);
>  	return false;
>  }
> +ALLOW_ERROR_INJECTION(btrfs_data_csum_ok, FALSE);

[Severity: Medium]
Does this bypass the error reporting logic it is intended to test?

The commit message states: "Testing the error handling path that responds
to such corruption, including any warning output and uspace notification..."

However, when fail_function overrides a function marked with
ALLOW_ERROR_INJECTION, it forces an immediate return without executing any
of the function body. 

This means the actual error reporting section inside btrfs_data_csum_ok()
is entirely skipped:

fs/btrfs/inode.c:btrfs_data_csum_ok() {
    [ ... ]
zeroit:
	btrfs_print_data_csum_error(inode, file_offset, csum, csum_expected,
				    bbio->mirror_num);
	if (dev)
		btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_CORRUPTION_ERRS);
    [ ... ]
}

Since the injected error causes the function to return false before this code
is reached, won't this skip the exact warning output and device stat
increments the patch is trying to validate?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724021229.47302-1-chensong_2000@126.com?part=2