Re: [PATCH] block: Fix inflight statistic for MQ submission with !elevator

[Jens Axboe <axboe@kernel.dk>]

On 8/31/20 7:18 PM, Ming Lei wrote:
> On Mon, Aug 31, 2020 at 11:37 PM Gabriel Krisman Bertazi
> <krisman@collabora.com> wrote:
>>
>> According to Documentation/block/stat.rst, inflight should not include
>> I/O requests that are in the queue but not yet dispatched to the device,
>> but blk-mq identifies as inflight any request that has a tag allocated,
>> which, for queues without elevator, happens at request allocation time
>> and before it is queued in the ctx (default case in blk_mq_submit_bio).
>>
>> A more precise approach would be to only consider requests with state
>> MQ_RQ_IN_FLIGHT.
>>
>> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
>> ---
>>  block/blk-mq.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 0015a1892153..997b3327eaa8 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -105,7 +105,7 @@ static bool blk_mq_check_inflight(struct blk_mq_hw_ctx *hctx,
>>  {
>>         struct mq_inflight *mi = priv;
>>
>> -       if (rq->part == mi->part)
>> +       if (rq->part == mi->part && rq->state == MQ_RQ_IN_FLIGHT)
>>                 mi->inflight[rq_data_dir(rq)]++;
> 
> The fix looks fine. However, we have helper of
> blk_mq_request_started() for this purpose.

Why does it look fine? Did you see the older commit I referenced? I'm
not saying the change is wrong per se, just that this is the behavior
we've always had, and making this change would deviate from that. As
Gabriel states in the follow up, it's either changing the documentation
or the patch.

When replying to patches that have had previous discussion, please
reference that when making followups. That makes for a more productive
discussion on how to proceed.

-- 
Jens Axboe