On Mon, May 20, 2019 at 01:11:41PM +0200, Christoph Hellwig wrote: > On Thu, May 16, 2019 at 09:17:04PM +0800, Ming Lei wrote: > > ll_merge_requests_fn() is only called from attempt_merge() in case > > that ELEVATOR_BACK_MERGE is returned from blk_try_req_merge(). However, > > for discard merge of both virtio_blk and nvme, ELEVATOR_DISCARD_MERGE is > > always returned from blk_try_req_merge() in attempt_merge(), so looks > > ll_merge_requests_fn() shouldn't be called for virtio_blk/nvme's discard > > request. Just wondering if you may explain a bit how the change on > > ll_merge_requests_fn() in this patch makes a difference on the above > > two commits? > > Good question. I've seen virtio overwriting its range, but I think > this might have been been with a series to actually decrement > nr_phys_segments for all cases where we can merge the tail and front > bvecs. So mainline probably doesn't see it unless someone calls > blk_recalc_rq_segments due to a partial completion or when using > dm-multipath. Thinking of it at least the latter seems like a real > possibily, although a rather unlikely use case. This patch shouldn't effect discard IO in case of partial completion too cause blk_recalc_rq_segments() always return 0 for discard IO w/wo this patch. However looks this way is wrong, the following patch may help for this case: diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 1aafeb923e7b..302667887ea1 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1109,7 +1109,12 @@ static inline unsigned short blk_rq_nr_phys_segments(struct request *rq) */ static inline unsigned short blk_rq_nr_discard_segments(struct request *rq) { - return max_t(unsigned short, rq->nr_phys_segments, 1); + struct bio *bio; + unsigned shart segs = 0; + + __rq_for_each_bio(bio, rq) + segs++; + return segs; } extern int blk_rq_map_sg(struct request_queue *, struct request *, struct scatterlist *); Or re-calculate the segment number in this way for multi-range discard IO in __blk_recalc_rq_segments(). Thanks, Ming