* [PATCH] block: fix inflight statistics of part0
@ 2020-11-26 9:48 Jeffle Xu
2020-11-30 6:46 ` JeffleXu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jeffle Xu @ 2020-11-26 9:48 UTC (permalink / raw)
To: axboe; +Cc: linux-block, joseph.qi
The inflight of partition 0 doesn't include inflight IOs to all
sub-partitions, since currently mq calculates inflight of specific
partition by simply camparing the value of the partition pointer.
Thus the following case is possible:
$ cat /sys/block/vda/inflight
0 0
$ cat /sys/block/vda/vda1/inflight
0 128
Partition 0 should be specially handled since it represents the whole
disk.
Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
block/blk-mq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 55bcee5dc032..04b6b4d21ce6 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -105,7 +105,8 @@ static bool blk_mq_check_inflight(struct blk_mq_hw_ctx *hctx,
{
struct mq_inflight *mi = priv;
- if (rq->part == mi->part && blk_mq_rq_state(rq) == MQ_RQ_IN_FLIGHT)
+ if ((!mi->part->partno || rq->part == mi->part) &&
+ blk_mq_rq_state(rq) == MQ_RQ_IN_FLIGHT)
mi->inflight[rq_data_dir(rq)]++;
return true;
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] block: fix inflight statistics of part0
2020-11-26 9:48 [PATCH] block: fix inflight statistics of part0 Jeffle Xu
@ 2020-11-30 6:46 ` JeffleXu
2020-11-30 17:05 ` Christoph Hellwig
2020-12-02 8:29 ` Christoph Hellwig
2 siblings, 0 replies; 6+ messages in thread
From: JeffleXu @ 2020-11-30 6:46 UTC (permalink / raw)
To: axboe; +Cc: linux-block, joseph.qi
Hello, any comment? Is this indeed a BUG or just a deliberate design?
--
Thanks,
Jeffle
On 11/26/20 5:48 PM, Jeffle Xu wrote:
> The inflight of partition 0 doesn't include inflight IOs to all
> sub-partitions, since currently mq calculates inflight of specific
> partition by simply camparing the value of the partition pointer.
>
> Thus the following case is possible:
>
> $ cat /sys/block/vda/inflight
> 0 0
> $ cat /sys/block/vda/vda1/inflight
> 0 128
>
> Partition 0 should be specially handled since it represents the whole
> disk.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
> block/blk-mq.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 55bcee5dc032..04b6b4d21ce6 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -105,7 +105,8 @@ static bool blk_mq_check_inflight(struct blk_mq_hw_ctx *hctx,
> {
> struct mq_inflight *mi = priv;
>
> - if (rq->part == mi->part && blk_mq_rq_state(rq) == MQ_RQ_IN_FLIGHT)
> + if ((!mi->part->partno || rq->part == mi->part) &&
> + blk_mq_rq_state(rq) == MQ_RQ_IN_FLIGHT)
> mi->inflight[rq_data_dir(rq)]++;
>
> return true;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] block: fix inflight statistics of part0
2020-11-26 9:48 [PATCH] block: fix inflight statistics of part0 Jeffle Xu
2020-11-30 6:46 ` JeffleXu
@ 2020-11-30 17:05 ` Christoph Hellwig
2020-12-01 1:53 ` JeffleXu
2020-12-02 8:29 ` Christoph Hellwig
2 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-11-30 17:05 UTC (permalink / raw)
To: Jeffle Xu; +Cc: axboe, linux-block, joseph.qi
On Thu, Nov 26, 2020 at 05:48:33PM +0800, Jeffle Xu wrote:
> The inflight of partition 0 doesn't include inflight IOs to all
> sub-partitions, since currently mq calculates inflight of specific
> partition by simply camparing the value of the partition pointer.
>
> Thus the following case is possible:
>
> $ cat /sys/block/vda/inflight
> ?? ?? ?? ??0 ?? ?? ?? ??0
> $ cat /sys/block/vda/vda1/inflight
> ?? ?? ?? ??0 ?? ?? ??128
>
> Partition 0 should be specially handled since it represents the whole
> disk.
I'm not sure and can see arguments for either side. In doubt we should
stick to historic behavior, can you check what old kernels (especially
before blk-mq) did?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] block: fix inflight statistics of part0
2020-11-30 17:05 ` Christoph Hellwig
@ 2020-12-01 1:53 ` JeffleXu
0 siblings, 0 replies; 6+ messages in thread
From: JeffleXu @ 2020-12-01 1:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: axboe, linux-block, joseph.qi
The traditional single queue block device has no problem. Following is
the output when I writes to sda3 on version v3.10.
$cat /sys/block/sda/sda3/inflight
0 33
$cat /sys/block/sda/inflight
0 33
On the other hand, we can analyze this from the code. Following code
path for single-queue block device is from v4.19.
1. When reading '/sys/block/sda/inflight', the statistics is actually
fetched from part 0.
part_inflight_show
part_in_flight_rw
inflight[0] = atomic_read(&part->in_flight[0]);
inflight[1] = atomic_read(&part->in_flight[1]);
2. part 0 will always be updated whenever sub partition is updated.
blk_queue_bio
add_acct_request
blk_account_io_start
part_inc_in_flight
atomic_inc(&part->in_flight[rw])
if (part->partno)
atomic_inc(part0.in_flight[rw]);
On 12/1/20 1:05 AM, Christoph Hellwig wrote:
> On Thu, Nov 26, 2020 at 05:48:33PM +0800, Jeffle Xu wrote:
>> The inflight of partition 0 doesn't include inflight IOs to all
>> sub-partitions, since currently mq calculates inflight of specific
>> partition by simply camparing the value of the partition pointer.
>>
>> Thus the following case is possible:
>>
>> $ cat /sys/block/vda/inflight
>> ?? ?? ?? ??0 ?? ?? ?? ??0
>> $ cat /sys/block/vda/vda1/inflight
>> ?? ?? ?? ??0 ?? ?? ??128
>>
>> Partition 0 should be specially handled since it represents the whole
>> disk.
>
> I'm not sure and can see arguments for either side. In doubt we should
> stick to historic behavior, can you check what old kernels (especially
> before blk-mq) did?
>
--
Thanks,
Jeffle
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] block: fix inflight statistics of part0
2020-11-26 9:48 [PATCH] block: fix inflight statistics of part0 Jeffle Xu
2020-11-30 6:46 ` JeffleXu
2020-11-30 17:05 ` Christoph Hellwig
@ 2020-12-02 8:29 ` Christoph Hellwig
2020-12-02 8:49 ` JeffleXu
2 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-12-02 8:29 UTC (permalink / raw)
To: Jeffle Xu; +Cc: axboe, linux-block, joseph.qi
On Thu, Nov 26, 2020 at 05:48:33PM +0800, Jeffle Xu wrote:
> The inflight of partition 0 doesn't include inflight IOs to all
> sub-partitions, since currently mq calculates inflight of specific
> partition by simply camparing the value of the partition pointer.
>
> Thus the following case is possible:
>
> $ cat /sys/block/vda/inflight
> ?? ?? ?? ??0 ?? ?? ?? ??0
> $ cat /sys/block/vda/vda1/inflight
> ?? ?? ?? ??0 ?? ?? ??128
>
> Partition 0 should be specially handled since it represents the whole
> disk.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
But can you resend with your information on the old kernels, and maybe
even with a Fixes tag?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] block: fix inflight statistics of part0
2020-12-02 8:29 ` Christoph Hellwig
@ 2020-12-02 8:49 ` JeffleXu
0 siblings, 0 replies; 6+ messages in thread
From: JeffleXu @ 2020-12-02 8:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: axboe, linux-block, joseph.qi
On 12/2/20 4:29 PM, Christoph Hellwig wrote:
> On Thu, Nov 26, 2020 at 05:48:33PM +0800, Jeffle Xu wrote:
>> The inflight of partition 0 doesn't include inflight IOs to all
>> sub-partitions, since currently mq calculates inflight of specific
>> partition by simply camparing the value of the partition pointer.
>>
>> Thus the following case is possible:
>>
>> $ cat /sys/block/vda/inflight
>> ?? ?? ?? ??0 ?? ?? ?? ??0
>> $ cat /sys/block/vda/vda1/inflight
>> ?? ?? ?? ??0 ?? ?? ??128
>>
>> Partition 0 should be specially handled since it represents the whole
>> disk.
>>
>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> But can you resend with your information on the old kernels, and maybe
> even with a Fixes tag?
>
Thanks, I will send a v2 version later.
--
Thanks,
Jeffle
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-02 8:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-26 9:48 [PATCH] block: fix inflight statistics of part0 Jeffle Xu
2020-11-30 6:46 ` JeffleXu
2020-11-30 17:05 ` Christoph Hellwig
2020-12-01 1:53 ` JeffleXu
2020-12-02 8:29 ` Christoph Hellwig
2020-12-02 8:49 ` JeffleXu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).