Re: [PATCH 1/2] block: modify blk_mq_plug() to allow only reads for zoned block devices

[Jens Axboe <axboe@kernel.dk>]

On 9/27/22 5:10 PM, Damien Le Moal wrote:
> On 9/28/22 08:07, Damien Le Moal wrote:
>> 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;
> 
> Note that we could also add a "IS_ENABLED(CONFIG_BLK_DEV_ZONED) &&" to
> the condition or stub the helper to have this hunk disappear for the
> !CONFIG_BLK_DEV_ZONED case.

Indeed, that would be nice.

-- 
Jens Axboe