On 9/28/22 01:52, Jens Axboe wrote:
> On 9/27/22 10:51 AM, Christoph Hellwig wrote:
>> On Tue, Sep 27, 2022 at 10:04:19AM -0600, Jens Axboe wrote:
>>> Ah yes, good point. We used to have this notion of 'fs' request, don't
>>> think we do anymore. Because it really should just be:
>>
>> A fs request is a !passthrough request.
>
> Right, that's the condition I made below too.
>
>>> if (zoned && (op & REQ_OP_WRITE) && fs_request)
>>> return NULL;
>>>
>>> for that condition imho. I guess we could make it:
>>>
>>> if (zoned && (op & REQ_OP_WRITE) && !(op & REQ_OP_DRV_OUT))
>>> return NULL;
>>
>> Well, the only opcodes we do zone locking for is REQ_OP_WRITE and
>> REQ_OP_WRITE_ZEROES. So this should be:
>>
>> if (zoned && (op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES))
>> return NULL;
>
> I'd rather just make it explicit and use that. Pankaj, do you want
> to spin a v2 with that?
It would be nice to reuse the bio equivalent of
blk_req_needs_zone_write_lock().
The test would be:
if (bio_needs_zone_write_locking())
return NULL;
With something like:
static inline bool bio_needs_zone_write_locking()
{
if (!bdev_is_zoned(bio->bi_bdev))
return false;
switch (bio_op(bio)) {
case REQ_OP_WRITE_ZEROES:
case REQ_OP_WRITE:
return true;
default:
return false;
}
}
Which also has the advantage that going forward, we could refine this to
plug writes to conventional zones (as these can be plugged).
--
Damien Le Moal
Western Digital Research
