On 8/27/24 07:26, Damien Le Moal wrote:
> On 8/27/24 02:37, Christoph Hellwig wrote:
>> The current setup with bio_may_exceed_limit and __bio_split_to_limits
>> is a bit of a mess.
>>
>> Change it so that __bio_split_to_limits does all the work and is just
>> a variant of bio_split_to_limits that returns nr_segs. This is done
>> by inlining it and instead have the various bio_split_* helpers directly
>> submit the potentially split bios.
>>
>> To support btrfs, the rw version has a lower level helper split out
>> that just returns the offset to split. This turns out to nicely clean
>> up the btrfs flow as well.
>>
>> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>> ---
>> block/blk-merge.c | 146 +++++++++++++++++---------------------------
>> block/blk-mq.c | 11 ++--
>> block/blk.h | 63 +++++++++++++------
>> fs/btrfs/bio.c | 30 +++++----
>> include/linux/bio.h | 4 +-
>> 5 files changed, 125 insertions(+), 129 deletions(-)
>>
>> diff --git a/block/blk-merge.c b/block/blk-merge.c
>> index de5281bcadc538..c7222c4685e060 100644
>> --- a/block/blk-merge.c
>> +++ b/block/blk-merge.c
>> @@ -105,9 +105,33 @@ static unsigned int bio_allowed_max_sectors(const struct queue_limits *lim)
>> return round_down(UINT_MAX, lim->logical_block_size) >> SECTOR_SHIFT;
>> }
>>
>> -static struct bio *bio_split_discard(struct bio *bio,
>> - const struct queue_limits *lim,
>> - unsigned *nsegs, struct bio_set *bs)
>> +static struct bio *bio_submit_split(struct bio *bio, int split_sectors)
>
> Why not "unsigned int" for split_sectors ? That would avoid the need for the
> first "if" of the function. Note that bio_split() also takes an int for the
> sector count and also checks for < 0 count with a BUG_ON(). We can clean that up
> too. BIOs sector count is unsigned int...
>
>> +{
>> + if (unlikely(split_sectors < 0)) {
>> + bio->bi_status = errno_to_blk_status(split_sectors);
>> + bio_endio(bio);
>> + return NULL;
>> + }
>> +
>> + if (split_sectors) {
>
> May be the simple case should come first ? E.g.:
>
> if (!split_sectors)
> return bio;
>
> But shouldn't this check be:
>
> if (split_sectors >= bio_sectors(bio))
> return bio;
Please ignore this one. The passed sector count is a limit, which can be 0, so
checking "if (!split_sectors)" is correct.
--
Damien Le Moal
Western Digital Research
