| 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 |
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; >
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.
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.
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.
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 --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;
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(-)