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

[Yu Kuai]

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