On Fri, Apr 28, 2023 at 12:46:06PM -0700, Bart Van Assche wrote:
>>> + if ((bio_op(bio) == REQ_OP_WRITE ||
>>> + bio_op(bio) == REQ_OP_WRITE_ZEROES) &&
>>> + disk_zone_is_seq(bio->bi_bdev->bd_disk, bio->bi_iter.bi_sector))
>>> return NULL;
>>
>> I find this a bit hard to hard to read. Why not:
>>
>> if (disk_zone_is_seq(bio->bi_bdev->bd_disk, bio->bi_iter.bi_sector)) {
>> /*
>> * Do not plug for writes that require zone locking.
>> */
>> if (bio_op(bio) == REQ_OP_WRITE ||
>> bio_op(bio) == REQ_OP_WRITE_ZEROES)
>> return NULL;
>> }
>
> In the above alternative the expensive check happens before a check that is not
> expensive at all. Do you really want me to call disk_zone_is_seq() before checking
> the operation type?
What expensive check? The first check in disk_zone_is_seq is for
a zoned device, avoiding any further check if it is not.
