* [RESEND PATCH v2] block: Consider only dispatched requests for inflight statistic
@ 2020-10-06 19:15 Gabriel Krisman Bertazi
2020-10-06 19:20 ` Gabriel Krisman Bertazi
0 siblings, 1 reply; 6+ messages in thread
From: Gabriel Krisman Bertazi @ 2020-10-06 19:15 UTC (permalink / raw)
To: axboe; +Cc: linux-block, ming.lei, Gabriel Krisman Bertazi, kernel,
Omar Sandoval
Hi Jens,
Reviving this, hopefully for the upcoming merge window, following the
suggestion that we let it sit there and see if people complain about the
metric fix. As I mentioned, I'm also happy to change the documentation
plus create a new in_device metric to track only requests submitted, if
you prefer.
Thanks,
>8
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).
In addition, current behavior is different for queues with elevator from
queues without it, since for the former the driver tag is allocated at
dispatch time. A more precise approach would be to only consider
requests with state MQ_RQ_IN_FLIGHT.
This effectively reverts commit 6131837b1de6 ("blk-mq: count allocated
but not started requests in iostats inflight") to consolidate blk-mq
behavior with itself (elevator case) and with original documentation,
but it differs from the behavior used by the legacy path.
This version differs from v1 by using blk_mq_rq_state to access the
state attribute. Avoid using blk_mq_request_started, which was
suggested, since we don't want to include MQ_RQ_COMPLETE.
Cc: Omar Sandoval <osandov@fb.com>
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 2e4b3cad2a61..c5fefd39d0c0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
--
2.28.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [RESEND PATCH v2] block: Consider only dispatched requests for inflight statistic
2020-10-06 19:15 [RESEND PATCH v2] block: Consider only dispatched requests for inflight statistic Gabriel Krisman Bertazi
@ 2020-10-06 19:20 ` Gabriel Krisman Bertazi
2020-10-06 19:27 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Gabriel Krisman Bertazi @ 2020-10-06 19:20 UTC (permalink / raw)
To: axboe; +Cc: linux-block, ming.lei, Gabriel Krisman Bertazi, kernel,
Omar Sandoval
Oops, I have no idea what happened, but something ate the hunk at the
last submission. My apologies. Please find it below.
>8
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).
In addition, current behavior is different for queues with elevator from
queues without it, since for the former the driver tag is allocated at
dispatch time. A more precise approach would be to only consider
requests with state MQ_RQ_IN_FLIGHT.
This effectively reverts commit 6131837b1de6 ("blk-mq: count allocated
but not started requests in iostats inflight") to consolidate blk-mq
behavior with itself (elevator case) and with original documentation,
but it differs from the behavior used by the legacy path.
This version differs from v1 by using blk_mq_rq_state to access the
state attribute. Avoid using blk_mq_request_started, which was
suggested, since we don't want to include MQ_RQ_COMPLETE.
Cc: Omar Sandoval <osandov@fb.com>
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 2e4b3cad2a61..c5fefd39d0c0 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 && blk_mq_rq_state(rq) == MQ_RQ_IN_FLIGHT)
mi->inflight[rq_data_dir(rq)]++;
return true;
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [RESEND PATCH v2] block: Consider only dispatched requests for inflight statistic
2020-10-06 19:20 ` Gabriel Krisman Bertazi
@ 2020-10-06 19:27 ` Jens Axboe
2020-10-06 19:33 ` Gabriel Krisman Bertazi
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2020-10-06 19:27 UTC (permalink / raw)
To: Gabriel Krisman Bertazi; +Cc: linux-block, ming.lei, kernel, Omar Sandoval
On 10/6/20 1:20 PM, Gabriel Krisman Bertazi wrote:
>
> Oops, I have no idea what happened, but something ate the hunk at the
> last submission. My apologies. Please find it below.
Care to just resend a fixed up one? Saves me the time from fixing
things up.
I guess we'll just try and see if this flies, not sure how else to
make progress on it.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH v2] block: Consider only dispatched requests for inflight statistic
2020-10-06 19:27 ` Jens Axboe
@ 2020-10-06 19:33 ` Gabriel Krisman Bertazi
2020-10-06 19:35 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Gabriel Krisman Bertazi @ 2020-10-06 19:33 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, ming.lei, kernel, Omar Sandoval
Jens Axboe <axboe@kernel.dk> writes:
> On 10/6/20 1:20 PM, Gabriel Krisman Bertazi wrote:
>>
>> Oops, I have no idea what happened, but something ate the hunk at the
>> last submission. My apologies. Please find it below.
>
> Care to just resend a fixed up one? Saves me the time from fixing
> things up.
hm, the first submission had an empty patch and the email you quoted had
the entire fixed patch ready to apply with scissors. It should be good
to apply it, I think. Or, what do you mean?
--
Gabriel Krisman Bertazi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH v2] block: Consider only dispatched requests for inflight statistic
2020-10-06 19:33 ` Gabriel Krisman Bertazi
@ 2020-10-06 19:35 ` Jens Axboe
2020-10-06 19:42 ` Gabriel Krisman Bertazi
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2020-10-06 19:35 UTC (permalink / raw)
To: Gabriel Krisman Bertazi; +Cc: linux-block, ming.lei, kernel, Omar Sandoval
On 10/6/20 1:33 PM, Gabriel Krisman Bertazi wrote:
> Jens Axboe <axboe@kernel.dk> writes:
>
>> On 10/6/20 1:20 PM, Gabriel Krisman Bertazi wrote:
>>>
>>> Oops, I have no idea what happened, but something ate the hunk at the
>>> last submission. My apologies. Please find it below.
>>
>> Care to just resend a fixed up one? Saves me the time from fixing
>> things up.
>
> hm, the first submission had an empty patch and the email you quoted had
> the entire fixed patch ready to apply with scissors. It should be good
> to apply it, I think. Or, what do you mean?
The point is that I need to manually fiddle with it.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH v2] block: Consider only dispatched requests for inflight statistic
2020-10-06 19:35 ` Jens Axboe
@ 2020-10-06 19:42 ` Gabriel Krisman Bertazi
0 siblings, 0 replies; 6+ messages in thread
From: Gabriel Krisman Bertazi @ 2020-10-06 19:42 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, ming.lei, kernel, Omar Sandoval
Jens Axboe <axboe@kernel.dk> writes:
> On 10/6/20 1:33 PM, Gabriel Krisman Bertazi wrote:
>> Jens Axboe <axboe@kernel.dk> writes:
>>
>>> On 10/6/20 1:20 PM, Gabriel Krisman Bertazi wrote:
>>>>
>>>> Oops, I have no idea what happened, but something ate the hunk at the
>>>> last submission. My apologies. Please find it below.
>>>
>>> Care to just resend a fixed up one? Saves me the time from fixing
>>> things up.
>>
>> hm, the first submission had an empty patch and the email you quoted had
>> the entire fixed patch ready to apply with scissors. It should be good
>> to apply it, I think. Or, what do you mean?
>
> The point is that I need to manually fiddle with it.
Done, sorry for the noise. The v3 should apply on top of your for-next
branch cleanly.
--
Gabriel Krisman Bertazi
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-06 19:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-06 19:15 [RESEND PATCH v2] block: Consider only dispatched requests for inflight statistic Gabriel Krisman Bertazi
2020-10-06 19:20 ` Gabriel Krisman Bertazi
2020-10-06 19:27 ` Jens Axboe
2020-10-06 19:33 ` Gabriel Krisman Bertazi
2020-10-06 19:35 ` Jens Axboe
2020-10-06 19:42 ` Gabriel Krisman Bertazi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.