> 2025年2月21日 18:09，Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> 写道：
>
> Hi,
>
> 在 2025/02/21 17:52, Coly Li 写道:
>> On Fri, Feb 21, 2025 at 04:11:02PM +0800, Zheng Qixing wrote:
>>> From: Li Nan <linan122@xxxxxxxxxx>
>>>
>>> _badblocks_set() returns success if at least one badblock is set
>>> successfully, even if others fail. This can lead to data inconsistencies
>>> in raid, where a failed badblock set should trigger the disk to be kicked
>>> out to prevent future reads from failed write areas.
>>>
>>> _badblocks_set() should return error if any badblock set fails. Instead
>>> of relying on 'rv', directly returning 'sectors' for clearer logic. If all
>>> badblocks are successfully set, 'sectors' will be 0, otherwise it
>>> indicates the number of badblocks that have not been set yet, thus
>>> signaling failure.
>>>
>>> By the way, it can also fix an issue: when a newly set unack badblock is
>>> included in an existing ack badblock, the setting will return an error.
>>> ···
>>> echo "0 100" /sys/block/md0/md/dev-loop1/bad_blocks
>>> echo "0 100" /sys/block/md0/md/dev-loop1/unacknowledged_bad_blocks
>>> -bash: echo: write error: No space left on device
>>> ```
>>> After fix, it will return success.
>>>
>>> Fixes: aa511ff8218b ("badblocks: switch to the improved badblock handling code")
>>> Signed-off-by: Li Nan <linan122@xxxxxxxxxx>
>>> ---
>>> block/badblocks.c | 16 ++++------------
>>> 1 file changed, 4 insertions(+), 12 deletions(-)
>>>
>> NACK. Such modification will break current API.
>
> Take a look at current APIs:
> - for raid, error should be returned, otherwise data may be corrupted.
> - for nvdimm, there is only error message if fail, and it make sense as
> well if any badblocks set failed:
> if (badblocks_set(bb, s, num, 1))
> dev_info_once(bb->dev, "%s: failed for sector %llx\n",
> __func__, (u64) s);
> - for null_blk, I think it's fine as well.
>
> Hence I think it's fine to return error if any badblocks set failed.
> There is no need to invent a new API and switch all callers to a new
> API.
So we don’t need to add a negative return value for partial success/failure?
Coly Li
