Re: [PATCH 0/2] block: Add done_split_bio() for chained bio post-processing

[Li kunyu]

On 2026/11/3 18:30, Christoph Hellwig wrote:
> On Fri, Oct 30, 2026 at 02:59:43PM +0800, Li kunyu wrote:
>> patch 1:  Introduce a new abstract function 'done_split_bio' and call it
>> from bio_remaining_done to resolve this issue.
>>
>> patch 2: Implement the blkcg_iolatency_done_split_bio callback in
>> blk-iolatency to correctly decrement the inflight count when the other
>> bio headers, which have not called rq_qos_done_bio(bio), complete.
> Why are you not simplify calling rq_qos_done_bio before
> bio_remaining_done?  __bi_remaining is only incremented for clones of
> various types, and the qos processing should probably happen before
> calling back into the submitter anyway.
> 
> As far as I can tell all the code here is duplicating the existing
> done code without much of an explanation of why we need another copy.
> 
> While we're at, the existing code in rq_qos_done_bio added in commit
> aa1b46dcdc7b ("block: fix rq-qos breakage from skipping rq_qos_done_bio()")
> is extremly wrong and dangerous.  Once a bio completes, bi_bdev
> is invalid, the driver could have even used it for something that is not
> a block device like md raid5 does, but it certainly does not have any
> use for the qos/iocost code.  So it really needs to derive anything
> it needs to I/O processing from the bio itself.


I identified this issue in the blk-iolatency framework under the QoS model: the
inflight accounting for split bios is incorrect. Specifically, inflight I/Os are
accumulated in the throttle function across the entire bio chain, whereas the
done_bio function subtracts only a portion of the head bio and child bios.

This patch may still have some issues; no anomalies were observed during testing
or in the scenarios exercised so far. For example, this submission adds the line
bio_clear_flag(bio, BIO_QOS_CHAIN_CHILD);.

I will continue to analyze whether a more appropriate fix can be found.

Thanks for the suggestion.

-- 
thanks,
kunyu