diff mbox series

[v2] block: fix inflight statistics of part0

Message ID 20201202111145.36000-1-jefflexu@linux.alibaba.com (mailing list archive)
State New, archived
Headers show
Series [v2] block: fix inflight statistics of part0 | expand

Commit Message

Jingbo Xu Dec. 2, 2020, 11:11 a.m. UTC
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

While single queue device (on a previous version, e.g. v3.10) has no
this issue:

$cat /sys/block/sda/sda3/inflight
       0       33
$cat /sys/block/sda/inflight
       0       33

Partition 0 should be specially handled since it represents the whole
disk. This issue is introduced since commit bf0ddaba65dd ("blk-mq: fix
sysfs inflight counter").

Besides, this patch can also fix the inflight statistics of part 0 in
/proc/diskstats. Before this patch, the inflight statistics of part 0
doesn't include that of sub partitions. (I have marked the 'inflight'
field with asterisk.)

$cat /proc/diskstats
 259       0 nvme0n1 45974469 0 367814768 6445794 1 0 1 0 *0* 111062 6445794 0 0 0 0 0 0
 259       2 nvme0n1p1 45974058 0 367797952 6445727 0 0 0 0 *33* 111001 6445727 0 0 0 0 0 0

This is introduced since commit f299b7c7a9de ("blk-mq: provide internal
in-flight variant").

Fixes: bf0ddaba65dd ("blk-mq: fix sysfs inflight counter")
Fixes: f299b7c7a9de ("blk-mq: provide internal in-flight variant")
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
v2: update the commit log, adding 'Fixes' tag
---
 block/blk-mq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jingbo Xu Dec. 2, 2020, 11:17 a.m. UTC | #1
On 12/2/20 7:11 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
> 
> While single queue device (on a previous version, e.g. v3.10) has no
> this issue:
> 
> $cat /sys/block/sda/sda3/inflight
>        0       33
> $cat /sys/block/sda/inflight
>        0       33
> 
> Partition 0 should be specially handled since it represents the whole
> disk. This issue is introduced since commit bf0ddaba65dd ("blk-mq: fix
> sysfs inflight counter").
> 
> Besides, this patch can also fix the inflight statistics of part 0 in
> /proc/diskstats. Before this patch, the inflight statistics of part 0
> doesn't include that of sub partitions. (I have marked the 'inflight'
> field with asterisk.)
> 
> $cat /proc/diskstats
>  259       0 nvme0n1 45974469 0 367814768 6445794 1 0 1 0 *0* 111062 6445794 0 0 0 0 0 0
>  259       2 nvme0n1p1 45974058 0 367797952 6445727 0 0 0 0 *33* 111001 6445727 0 0 0 0 0 0
> 
> This is introduced since commit f299b7c7a9de ("blk-mq: provide internal
> in-flight variant").
> 
> Fixes: bf0ddaba65dd ("blk-mq: fix sysfs inflight counter")
> Fixes: f299b7c7a9de ("blk-mq: provide internal in-flight variant")
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
> v2: update the commit log, adding 'Fixes' tag

Forgot to add 'stable' tag.

> ---
>  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;
>
Christoph Hellwig Dec. 2, 2020, 11:20 a.m. UTC | #2
On Wed, Dec 02, 2020 at 07:17:55PM +0800, JeffleXu wrote:
> > Fixes: bf0ddaba65dd ("blk-mq: fix sysfs inflight counter")
> > Fixes: f299b7c7a9de ("blk-mq: provide internal in-flight variant")
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> > ---
> > v2: update the commit log, adding 'Fixes' tag
> 
> Forgot to add 'stable' tag.

The fixes tags take care of that automatically.

Note that this patch will cause a merge conflict with my work in
linux-next, but the resolution is pretty trivial.
Jens Axboe Dec. 2, 2020, 4:48 p.m. UTC | #3
On 12/2/20 4:20 AM, Christoph Hellwig wrote:
> On Wed, Dec 02, 2020 at 07:17:55PM +0800, JeffleXu wrote:
>>> Fixes: bf0ddaba65dd ("blk-mq: fix sysfs inflight counter")
>>> Fixes: f299b7c7a9de ("blk-mq: provide internal in-flight variant")
>>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
>>> ---
>>> v2: update the commit log, adding 'Fixes' tag
>>
>> Forgot to add 'stable' tag.
> 
> The fixes tags take care of that automatically.
> 
> Note that this patch will cause a merge conflict with my work in
> linux-next, but the resolution is pretty trivial.

Might be better to handle on the stable side, and just apply this to
5.11. It's not a new regression.
Christoph Hellwig Dec. 2, 2020, 7:05 p.m. UTC | #4
On Wed, Dec 02, 2020 at 09:48:03AM -0700, Jens Axboe wrote:
> On 12/2/20 4:20 AM, Christoph Hellwig wrote:
> > On Wed, Dec 02, 2020 at 07:17:55PM +0800, JeffleXu wrote:
> >>> Fixes: bf0ddaba65dd ("blk-mq: fix sysfs inflight counter")
> >>> Fixes: f299b7c7a9de ("blk-mq: provide internal in-flight variant")
> >>> Reviewed-by: Christoph Hellwig <hch@lst.de>
> >>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> >>> ---
> >>> v2: update the commit log, adding 'Fixes' tag
> >>
> >> Forgot to add 'stable' tag.
> > 
> > The fixes tags take care of that automatically.
> > 
> > Note that this patch will cause a merge conflict with my work in
> > linux-next, but the resolution is pretty trivial.
> 
> Might be better to handle on the stable side, and just apply this to
> 5.11. It's not a new regression.

If you apply it to for-5.11/block it won't compile.  It'll need a quick
s/partno/bd_partno/ there.
Jingbo Xu Dec. 3, 2020, 1:46 a.m. UTC | #5
On 12/3/20 3:05 AM, Christoph Hellwig wrote:
> On Wed, Dec 02, 2020 at 09:48:03AM -0700, Jens Axboe wrote:
>> On 12/2/20 4:20 AM, Christoph Hellwig wrote:
>>> On Wed, Dec 02, 2020 at 07:17:55PM +0800, JeffleXu wrote:
>>>>> Fixes: bf0ddaba65dd ("blk-mq: fix sysfs inflight counter")
>>>>> Fixes: f299b7c7a9de ("blk-mq: provide internal in-flight variant")
>>>>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>>>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
>>>>> ---
>>>>> v2: update the commit log, adding 'Fixes' tag
>>>>
>>>> Forgot to add 'stable' tag.
>>>
>>> The fixes tags take care of that automatically.
>>>
>>> Note that this patch will cause a merge conflict with my work in
>>> linux-next, but the resolution is pretty trivial.
>>
>> Might be better to handle on the stable side, and just apply this to
>> 5.11. It's not a new regression.
> 
> If you apply it to for-5.11/block it won't compile.  It'll need a quick
> s/partno/bd_partno/ there.
> 

Fine. I will resend a new version later on the code base of for-5.11/block.
diff mbox series

Patch

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;