| Message ID | 20240911201240.3982856-5-kbusch@meta.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | block integrity merging and counting | expand |
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.
diff --git a/block/blk-integrity.c b/block/blk-integrity.c index afd101555d3cb..84065691aaed0 100644 --- a/block/blk-integrity.c +++ b/block/blk-integrity.c @@ -134,7 +134,6 @@ bool blk_integrity_merge_bio(struct request_queue *q, struct request *req, struct bio *bio) { int nr_integrity_segs; - struct bio *next = bio->bi_next; if (blk_integrity_rq(req) == 0 && bio_integrity(bio) == NULL) return true; @@ -145,10 +144,7 @@ bool blk_integrity_merge_bio(struct request_queue *q, struct request *req, if (bio_integrity(req->bio)->bip_flags != bio_integrity(bio)->bip_flags) return false; - bio->bi_next = NULL; nr_integrity_segs = blk_rq_count_integrity_sg(q, bio); - bio->bi_next = next; - if (req->nr_integrity_segments + nr_integrity_segs > q->limits.max_integrity_segments) return false;