On 4/3/24 03:26, Hannes Reinecke wrote:
>> diff --git a/block/bio.c b/block/bio.c
>> index d24420ed1c4c..4ece8cef1fbe 100644
>> --- a/block/bio.c
>> +++ b/block/bio.c
>> @@ -1576,6 +1576,13 @@ void bio_endio(struct bio *bio)
>> if (!bio_integrity_endio(bio))
>> return;
>> + /*
>> + * For BIOs handled through a zone write plug, signal the completion
>> + * of the BIO so that the next plugged BIO can be submitted.
>> + */
>> + if (bio_zone_write_plugging(bio))
>> + blk_zone_write_plug_bio_endio(bio);
>> +
>
> Can't we move this check into blk_zone_write_plug_bio_endio()?
> We'd need to check it anyway ...
The goal here is to avoid a useless function call for regular devices or read
BIOs. There is no double check as the flag is not checked again in
blk_zone_write_plug_bio_endio(). Same comment for the other points you raised
(except the last one, see below).
If you really insist, I could play games with inline functions to "hide" the
check though.
>> +/*
>> + * Called from bio_attempt_back_merge() when a BIO was merged with a request.
>> + */
>> +void blk_zone_write_plug_bio_merged(struct bio *bio)
>> +{
>> + struct blk_zone_wplug *zwplug;
>> + unsigned long flags;
>> +
>> + /*
>> + * If the BIO was already plugged, then we were called through
>> + * blk_zone_write_plug_attempt_merge() -> blk_attempt_bio_merge().
>> + * For this case, blk_zone_write_plug_attempt_merge() will handle the
>> + * zone write pointer offset update.
>> + */
>> + if (bio_flagged(bio, BIO_ZONE_WRITE_PLUGGING))
>> + return;
>> +
> See? you have to check anyway ...
This is the only function checking again, and for a good reason as the comment
above the check explains.
>
> Cheers,
>
> Hannes
--
Damien Le Moal
Western Digital Research
