Re: [PATCHv4 04/10] blk-integrity: consider entire bio list for merging

[Christoph Hellwig <hch@lst.de>]

On Wed, Sep 11, 2024 at 01:12:34PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> If a bio is merged to a request, the entire bio list is merged, so don't
> temporarily detach it from its list when counting segments. In most
> cases, bi_next will already be NULL, so detaching is usually a no-op.
> But if the bio does have a list, the current code is miscounting the
> segments for the resulting merge.

As far as I can tell we can never get here with bi_next set.  Rationale:

blk_integrity_merge_bio has two callers: ll_new_hw_segment and
blk_rq_merge_ok.

ll_new_hw_segment is called from ll_back_merge_fn and ll_front_merge_fn.

ll_back_merge_fn is called from blk_rq_append_bio and
bio_attempt_back_merge.

blk_rq_append_bio is always used for a single bio and in fact used
to build bio chains.

bio_attempt_back_merge is called from blk_attempt_bio_merge,
blk_mq_sched_try_merge and blk_zone_write_plug_init_request.

blk_attempt_bio_merge is called from blk_attempt_plug_merge and
blk_bio_list_merge.

blk_attempt_plug_merge is called from blk_mq_attempt_bio_merge,
which always operates on a single bio.

blk_bio_list_merge is called from blk_mq_sched_bio_merge,
kyber_bio_merge where the latter is just an indirect call from
the former, and blk_mq_sched_bio_merge is called from
blk_mq_attempt_bio_merge which was considered above.

blk_mq_sched_try_merge is called from bfq_bio_merge and dd_bio_merge,
both of which are implementation of the ->bio_merge elevator_mq_ops
method called from blk_mq_sched_bio_merge, which was considered above.

blk_zone_write_plug_init_request is called from blk_mq_submit_bio
and always operates on a single bio.

ll_front_merge_fn is called from bio_attempt_front_merge.

bio_attempt_front_merge is called from blk_attempt_bio_merge and
blk_mq_sched_try_merge, both of which were considered above.

blk_rq_merge_ok is called from blk_attempt_bio_merge,
blk_zone_write_plug_init_request and elv_bio_merge_ok.

blk_attempt_bio_merge and blk_zone_write_plug_init_request were
already considered above.

elv_bio_merge_ok is called from bfq_request_merge and dd_request_merge
and elv_merge.  The first two are implementations of the ->request_merge
elevator_mq_ops method called from elv_merge, which is called from
blk_mq_sched_try_merge, which was considered above.

Also it feels like the call in blk_rq_merge_ok is superflous from this.