On 1/10/23 18:52, Niklas Cassel wrote:
> On Mon, Jan 09, 2023 at 03:52:23PM -0800, Bart Van Assche wrote:
>> On 1/9/23 15:38, Damien Le Moal wrote:
>>> On 1/10/23 08:27, Bart Van Assche wrote:
>>>> +static inline bool blk_rq_is_seq_zone_write(struct request *rq)
>>>> +{
>>>> + switch (req_op(rq)) {
>>>> + case REQ_OP_WRITE:
>>>> + case REQ_OP_WRITE_ZEROES:
>>>
>>> REQ_OP_ZONE_APPEND ?
>>
>> I will add REQ_OP_ZONE_APPEND.
>>
>
> Hello Bart, Damien,
>
> + if (blk_queue_pipeline_zoned_writes(rq->q) &&
> + blk_rq_is_seq_zone_write(rq))
> + cmd->allowed += rq->q->nr_requests;
>
> Considering that this function, blk_rq_is_seq_zone_write(), only seems to
> be used to determine if a request should be allowed to be retried, I think
> that it is incorrect to add REQ_OP_ZONE_APPEND, since a zone append
> operation will never result in a ILLEGAL REQUEST/UNALIGNED WRITE COMMAND.
>
> (If this instead was a function that said which operations that needed to
> be held back, then you would probably need to include REQ_OP_ZONE_APPEND,
> as otherwise the reordered+retried write would never be able to succeed.)
Unless UFS defines a zone append operation, REQ_OP_ZONE_APPEND will be
processed using regular writes in the sd driver.
I suggested adding it given the name of the function. REQ_OP_ZONE_APPEND
is a zone write operation for sequential zones...
>
>
> Kind regards,
> Niklas
--
Damien Le Moal
Western Digital Research
