Re: [PATCHv5 1/2] block: accumulate memory segment gaps per bio

Yu Kuai
2025-11-11T10:14:34Z

Hi,

在 2025/11/11 17:39, Christoph Hellwig 写道:
> On Tue, Nov 11, 2025 at 05:36:39PM +0800, Yu Kuai wrote:
>> This can be reproduced 100% with branch for-6.19/block now, just:
>>
>> blkdiscard /dev/md0
>>
>> Where discard IO will be split to different underlying disks and then
>> merge. And for discard bio, bio->bi_io_vec is NULL. So when discard
>> bio ends up to the merge path, bio->bi_io_vec will be dereferenced
>> unconditionally.
> Ah, so it's not a NULL req->bio but bio->bi_io_vec.
>
>> How about following simple fix:
>>
>> diff --git a/block/blk-merge.c b/block/blk-merge.c
>> index 3ca6fbf8b787..31f460422fe3 100644
>> --- a/block/blk-merge.c
>> +++ b/block/blk-merge.c
>> @@ -740,6 +740,9 @@ u8 bio_seg_gap(struct request_queue *q, struct bio *prev, struct bio *next,
>> gaps_bit = min_not_zero(gaps_bit, prev->bi_bvec_gap_bit);
>> gaps_bit = min_not_zero(gaps_bit, next->bi_bvec_gap_bit);
>>
>> + if (op_is_discard(prev->bi_opf) || op_is_discard(next->bi_opf))
>> + return gaps_bit;
>> +
> I think the problem is how we even end up here? The only merging
> for discard should be the special multi-segment merge. So I think
> something higher up is messed up
>
At least from blk_try_merge(), blk_discard_mergable() do return false,
however, following checking passed and we end up to the back merge patch.

blk_try_merge
if (blk_discard_mergable())
// false due to max_discard_segments is 1
else if (...)
return ELEVATOR_BACK_MERGE

Perhaps are you suggesting to change this to:

enum elv_merge blk_try_merge(struct request *rq, struct bio *bio)
{
- if (blk_discard_mergable(rq))
- return ELEVATOR_DISCARD_MERGE;
- else if (blk_rq_pos(rq) + blk_rq_sectors(rq) == bio->bi_iter.bi_sector)
+ if (req_op(rq) == REQ_OP_DISCARD) {
+ if (blk_discard_mergable((rq)))
+ return ELEVATOR_DISCARD_MERGE;
+ return ELEVATOR_NO_MERGE;
+ }
+
+ if (blk_rq_pos(rq) + blk_rq_sectors(rq) == bio->bi_iter.bi_sector)
return ELEVATOR_BACK_MERGE;
- else if (blk_rq_pos(rq) - bio_sectors(bio) == bio->bi_iter.bi_sector)
+ if (blk_rq_pos(rq) - bio_sectors(bio) == bio->bi_iter.bi_sector)
return ELEVATOR_FRONT_MERGE;
return ELEVATOR_NO_MERGE;
}

And the same for other callers for blk_discard_mergable().

Thanks,
Kuai