[PATCH] blk-throttle: avoid multiple counting for same bio

[PATCH] blk-throttle: avoid multiple counting for same bio

[Chengguang Xu]

In current throttling/upper limit policy of blkio cgroup
blkio.throttle.io_service_bytes does not exactly represent
the number of bytes issued to the disk by the group, sometimes
this number could be counted multiple times of real bytes.
This fix introduces BIO_COUNTED flag to avoid multiple counting
for same bio.

Signed-off-by: Chengguang Xu <cgxu519@icloud.com>
---
 include/linux/blk-cgroup.h | 4 ++++
 include/linux/blk_types.h  | 1 +
 2 files changed, 5 insertions(+)

diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index 69bea82..6c77711 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -686,6 +686,9 @@ static inline bool blkcg_bio_issue_check(struct request_queue *q,
 	struct blkcg_gq *blkg;
 	bool throtl = false;
 
+	if (bio_flagged(bio, BIO_COUNTED))
+		return !throtl;
+
 	rcu_read_lock();
 	blkcg = bio_blkcg(bio);
 
@@ -708,6 +711,7 @@ static inline bool blkcg_bio_issue_check(struct request_queue *q,
 		blkg_rwstat_add(&blkg->stat_bytes, bio->bi_opf,
 				bio->bi_iter.bi_size);
 		blkg_rwstat_add(&blkg->stat_ios, bio->bi_opf, 1);
+		bio_set_flag(bio, BIO_COUNTED);
 	}
 
 	rcu_read_unlock();
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index bf18b95..23b83e7 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -181,6 +181,7 @@ struct bio {
 				 * throttling rules. Don't do it again. */
 #define BIO_TRACE_COMPLETION 10	/* bio_endio() should trace the final completion
 				 * of this bio. */
+#define BIO_COUNTED	11	/* This bio has already been counted. */
 /* See BVEC_POOL_OFFSET below before adding new flags */
 
 /*
-- 
1.8.3.1

Re: [PATCH] blk-throttle: avoid multiple counting for same bio

[Tejun Heo]

On Tue, Feb 13, 2018 at 02:45:50PM +0800, Chengguang Xu wrote:
> In current throttling/upper limit policy of blkio cgroup
> blkio.throttle.io_service_bytes does not exactly represent
> the number of bytes issued to the disk by the group, sometimes
> this number could be counted multiple times of real bytes.
> This fix introduces BIO_COUNTED flag to avoid multiple counting
> for same bio.
> 
> Signed-off-by: Chengguang Xu <cgxu519@icloud.com>

We had a series of fixes / changes for this problem during the last
cycle.  Can you please see whether the current linus master has the
same problem.

Thanks.

-- 
tejun

Re: [PATCH] blk-throttle: avoid multiple counting for same bio

[Tejun Heo]

On Tue, Feb 13, 2018 at 02:45:50PM +0800, Chengguang Xu wrote:
> In current throttling/upper limit policy of blkio cgroup
> blkio.throttle.io_service_bytes does not exactly represent
> the number of bytes issued to the disk by the group, sometimes
> this number could be counted multiple times of real bytes.
> This fix introduces BIO_COUNTED flag to avoid multiple counting
> for same bio.
> 
> Signed-off-by: Chengguang Xu <cgxu519-mVuRI66OGLPQT0dZR+AlfA@public.gmane.org>

We had a series of fixes / changes for this problem during the last
cycle.  Can you please see whether the current linus master has the
same problem.

Thanks.

-- 
tejun

Re: [PATCH] blk-throttle: avoid multiple counting for same bio

[Chengguang Xu]

Hi Tejun,

Sorry for delayed reply, I was on vacation last week.

The problem still exists in current code of 4.16.0-rc2,=20
detail test information is below, if further info is needed please let =
me know.

Thanks.

=
=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=
=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=
=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=
=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=
=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=
=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94
Both read/write bps are limited to 10MB/s in blkio cgroup v1 & v2

$ uname -r
4.16.0-rc2+


[Without this patch]----------------------------------------

CGROUP V1 (direct write):

$ dd if=3D/dev/zero of=3D/mnt/sdb1/20/test bs=3D1M count=3D1024 =
oflag=3Ddirect
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 102.402 s, 10.5 MB/s

8:16 Read 16384
8:16 Write 2684354560
8:16 Sync 2684370944
8:16 Async 0
8:16 Total 2684370944

CGROUP V1 (read):

$ dd if=3D/mnt/sdb1/20/test of=3D/dev/zero bs=3D1M count=3D1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 102.412 s, 10.5 MB/s

8:16 Read 4831838208
8:16 Write 0
8:16 Sync 4831838208
8:16 Async 0
8:16 Total 4831838208


CGROUP V2 (direct write):

$ cat io.max
8:16 rbps=3Dmax wbps=3D10485760 riops=3Dmax wiops=3Dmax

$ dd if=3D/dev/zero of=3D/mnt/sdb1/20/test bs=3D1M count=3D1024 =
oflag=3Ddirect
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 102.408 s, 10.5 MB/s

8:16 rbytes=3D24576 wbytes=3D2684354560 rios=3D5 wios=3D4096


CGROUP V2 (buffered write):

$ dd if=3D/dev/zero of=3D/mnt/sdb1/20/test bs=3D1M count=3D1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 0.637822 s, 1.7 GB/s

8:16 rbytes=3D0 wbytes=3D4831838208 rios=3D0 wios=3D4096

CGROUP V2 (read):

$ cat io.max
8:16 rbps=3D10485760 wbps=3Dmax riops=3Dmax wiops=3Dmax

$ dd if=3D/mnt/sdb1/20/test of=3D/dev/zero bs=3D1M count=3D1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 102.409 s, 10.5 MB/s

8:16 rbytes=3D4831846400 wbytes=3D0 rios=3D4097 wios=3D0

[With this patch]----------------------------------------

CGROUP V1 (direct write):

$ dd if=3D/dev/zero of=3D/mnt/sdb1/20/test bs=3D1M count=3D1024 =
oflag=3Ddirect
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 102.402 s, 10.5 MB/s

8:16 Read 24576
8:16 Write 1073741824
8:16 Sync 1073766400
8:16 Async 0
8:16 Total 1073766400

CGROUP V1 (read):

$ dd if=3D/mnt/sdb1/20/test of=3D/dev/zero bs=3D1M count=3D1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 102.406 s, 10.5 MB/s

8:16 Read 1073741824
8:16 Write 0
8:16 Sync 1073741824
8:16 Async 0
8:16 Total 1073741824

CGROUP V2 (direct write):

$ dd if=3D/dev/zero of=3D/mnt/sdb1/20/test bs=3D1M count=3D1024 =
oflag=3Ddirect
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 102.407 s, 10.5 MB/s

8:16 rbytes=3D16384 wbytes=3D1073741824 rios=3D4 wios=3D1024


CGROUP V2 (buffered write):

$ dd if=3D/dev/zero of=3D/mnt/sdb1/20/test bs=3D1M count=3D1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 0.650783 s, 1.6 GB/s

8:16 rbytes=3D0 wbytes=3D1073741824 rios=3D0 wios=3D512

CGROUP V2 (read):

$ dd if=3D/mnt/sdb1/20/test of=3D/dev/zero bs=3D1M count=3D1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 102.411 s, 10.5 MB/s

8:16 rbytes=3D1077314048 wbytes=3D0 rios=3D572 wios=3D0


=
=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=
=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=
=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=
=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=
=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=
=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94


> =E5=9C=A8 2018=E5=B9=B42=E6=9C=8813=E6=97=A5=EF=BC=8C=E4=B8=8B=E5=8D=881=
0:43=EF=BC=8CTejun Heo <tj@kernel.org> =E5=86=99=E9=81=93=EF=BC=9A
>=20
> On Tue, Feb 13, 2018 at 02:45:50PM +0800, Chengguang Xu wrote:
>> In current throttling/upper limit policy of blkio cgroup
>> blkio.throttle.io_service_bytes does not exactly represent
>> the number of bytes issued to the disk by the group, sometimes
>> this number could be counted multiple times of real bytes.
>> This fix introduces BIO_COUNTED flag to avoid multiple counting
>> for same bio.
>>=20
>> Signed-off-by: Chengguang Xu <cgxu519@icloud.com>
>=20
> We had a series of fixes / changes for this problem during the last
> cycle.  Can you please see whether the current linus master has the
> same problem.
>=20
> Thanks.
>=20
> --=20
> tejun

Re: [PATCH] blk-throttle: avoid multiple counting for same bio

[Joseph Qi]

On 18/2/23 11:33, Chengguang Xu wrote:
> Hi Tejun,
> 
> Sorry for delayed reply, I was on vacation last week.
> 
> The problem still exists in current code of 4.16.0-rc2, 
> detail test information is below, if further info is needed please let me know.
> 
> Thanks.
> 
That's true, the issue Shaohua has fixed is double charge, but double
stat issue still exists.

Jiufei has posted a fix, which has already been tested by Bo Liu:
[PATCH RESEND] blk-throttle: avoid double counted
https://www.mail-archive.com/linux-block@vger.kernel.org/msg18516.html

Thanks,
Joseph

Re: [PATCH] blk-throttle: avoid multiple counting for same bio

[Chengguang Xu]

Hi Joseph,

Thanks for quick reply, I didn=A1=AFt notice that patch before.


Thanks,
Chengguang.


> =D4=DA 2018=C4=EA2=D4=C223=C8=D5=A3=AC=C9=CF=CE=E711:55=A3=ACJoseph Qi =
<jiangqi903@gmail.com> =D0=B4=B5=C0=A3=BA
>=20
>=20
>=20
> On 18/2/23 11:33, Chengguang Xu wrote:
>> Hi Tejun,
>>=20
>> Sorry for delayed reply, I was on vacation last week.
>>=20
>> The problem still exists in current code of 4.16.0-rc2,=20
>> detail test information is below, if further info is needed please =
let me know.
>>=20
>> Thanks.
>>=20
> That's true, the issue Shaohua has fixed is double charge, but double
> stat issue still exists.
>=20
> Jiufei has posted a fix, which has already been tested by Bo Liu:
> [PATCH RESEND] blk-throttle: avoid double counted
> https://www.mail-archive.com/linux-block@vger.kernel.org/msg18516.html
>=20
> Thanks,
> Joseph
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] blk-throttle: avoid multiple counting for same bio

[Tejun Heo]

Hello,

On Fri, Feb 23, 2018 at 12:29:25PM +0800, Chengguang Xu wrote:
> > That's true, the issue Shaohua has fixed is double charge, but double
> > stat issue still exists.
> > 
> > Jiufei has posted a fix, which has already been tested by Bo Liu:
> > [PATCH RESEND] blk-throttle: avoid double counted
> > https://www.mail-archive.com/linux-block@vger.kernel.org/msg18516.html

Shaohua, can you please take a look at the patch?

Thanks.

-- 
tejun