Re: [PATCH 2/2] md: split bio by io_opt size in md_submit_bio()

[John Garry <john.g.garry@oracle.com>]

On 18/08/2025 11:26, Coly Li wrote:
>>> static struct bio *bio_split_by_io_opt(struct bio *bio)
>>> +{
>>> + sector_t io_opt_sectors, start, offset;
>>> + struct queue_limits lim;
>>> + struct mddev *mddev;
>>> + struct bio *split;
>>> + int level;
>>> +
>>> + mddev = bio->bi_bdev->bd_disk->private_data;
>>> + level = mddev->level;
>>> +
>>> + /* Only handle read456 read/write requests */
>>> + if (level == 1 || level == 10 || level == 0 || level == LEVEL_LINEAR ||
>>> +     (bio_op(bio) != REQ_OP_READ && bio_op(bio) != REQ_OP_WRITE))
>>> + return bio_split_to_limits(bio);
>> this should be taken outside this function, as we are not splitting to io_opt here
>>
> 
> It is not split to io_opt, it is split to max_hw_sectors. And the value of max_hw_sectors is aligned to io_opt.
> 
> 

Where is alignment of max_hw_sectors and io_opt enforced? raid1 does not 
even explicitly set max_hw_sectors or io_opt for the top device. I also 
note that raid10 does not set max_hw_sectors, which I doubt is proper. 
And md-linear does not set io_opt AFAICS.

> 
>>> +
>>> + /* In case raid456 chunk size is too large */
>>> + lim = mddev->gendisk->queue->limits;
>>> + io_opt_sectors = lim.io_opt >> SECTOR_SHIFT;
>>> + if (unlikely(io_opt_sectors > lim.max_hw_sectors))
>>> + return bio_split_to_limits(bio);
>>> +
>>> + /* Small request, no need to split */
>>> + if (bio_sectors(bio) <= io_opt_sectors)
>>> + return bio;
>> According to 1, above, we should split this if bio->bi_iter.bi_sector is not aligned, yet we possibly don't here
>>
> The split is only for performance, for too small bio, split or not doesn’t matter obviously for performance.
Then it should be part of the documented rules in the commit message.

Thanks,
John