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

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

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