From: Yu Kuai <yukuai3@huawei.com> To: <tj@kernel.org>, <axboe@kernel.dk>, <ming.lei@redhat.com> Cc: <cgroups@vger.kernel.org>, <linux-block@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <yukuai3@huawei.com>, <yi.zhang@huawei.com> Subject: [PATCH -next] blk-throttle: delay the setting of 'BIO_THROTTLED' to when throttle is done Date: Tue, 17 May 2022 21:49:09 +0800 [thread overview] Message-ID: <20220517134909.2910251-1-yukuai3@huawei.com> (raw) commit 9f5ede3c01f9 ("block: throttle split bio in case of iops limit") introduce a new problem, for example: [root@localhost ~]# echo "8:0 1024" > /sys/fs/cgroup/blkio/blkio.throttle.write_bps_device [root@localhost ~]# echo $$ > /sys/fs/cgroup/blkio/cgroup.procs [root@localhost ~]# dd if=/dev/zero of=/dev/sda bs=10k count=1 oflag=direct & [1] 620 [root@localhost ~]# dd if=/dev/zero of=/dev/sda bs=10k count=1 oflag=direct & [2] 626 [root@localhost ~]# 1+0 records in 1+0 records out 10240 bytes (10 kB, 10 KiB) copied, 10.0038 s, 1.0 kB/s1+0 records in 1+0 records out 10240 bytes (10 kB, 10 KiB) copied, 9.23076 s, 1.1 kB/s -> the second bio is issued after 10s instead of 20s. This is because if some bios are already queued, current bio is queued directly and the flag 'BIO_THROTTLED' is set. And later, when former bios are dispatched, this bio will be dispatched without waiting at all, this is due to tg_with_in_bps_limit() will return 0 if the flag is set. Instead of setting the flag when bio starts throttle, delay to when throttle is done to fix the problem. Fixes: 9f5ede3c01f9 ("block: throttle split bio in case of iops limit") Signed-off-by: Yu Kuai <yukuai3@huawei.com> --- block/blk-throttle.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 447e1b8722f7..f952f2d942ff 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -811,7 +811,7 @@ static bool tg_with_in_bps_limit(struct throtl_grp *tg, struct bio *bio, unsigned int bio_size = throtl_bio_data_size(bio); /* no need to throttle if this bio's bytes have been accounted */ - if (bps_limit == U64_MAX || bio_flagged(bio, BIO_THROTTLED)) { + if (bps_limit == U64_MAX) { if (wait) *wait = 0; return true; @@ -1226,8 +1226,10 @@ static void blk_throtl_dispatch_work_fn(struct work_struct *work) spin_lock_irq(&q->queue_lock); for (rw = READ; rw <= WRITE; rw++) - while ((bio = throtl_pop_queued(&td_sq->queued[rw], NULL))) + while ((bio = throtl_pop_queued(&td_sq->queued[rw], NULL))) { + bio_set_flag(bio, BIO_THROTTLED); bio_list_add(&bio_list_on_stack, bio); + } spin_unlock_irq(&q->queue_lock); if (!bio_list_empty(&bio_list_on_stack)) { @@ -2134,7 +2136,8 @@ bool __blk_throtl_bio(struct bio *bio) } break; } - + /* this bio will be issued directly */ + bio_set_flag(bio, BIO_THROTTLED); /* within limits, let's charge and dispatch directly */ throtl_charge_bio(tg, bio); @@ -2190,7 +2193,6 @@ bool __blk_throtl_bio(struct bio *bio) out_unlock: spin_unlock_irq(&q->queue_lock); - bio_set_flag(bio, BIO_THROTTLED); #ifdef CONFIG_BLK_DEV_THROTTLING_LOW if (throttled || !td->track_bio_latency) -- 2.31.1
WARNING: multiple messages have this Message-ID (diff)
From: Yu Kuai <yukuai3@huawei.com> To: tj@kernel.org, axboe@kernel.dk, ming.lei@redhat.com Cc: cgroups@vger.kernel.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, yukuai3@huawei.com, yi.zhang@huawei.com Subject: [PATCH -next] blk-throttle: delay the setting of 'BIO_THROTTLED' to when throttle is done Date: Tue, 17 May 2022 21:49:09 +0800 [thread overview] Message-ID: <20220517134909.2910251-1-yukuai3@huawei.com> (raw) commit 9f5ede3c01f9 ("block: throttle split bio in case of iops limit") introduce a new problem, for example: [root@localhost ~]# echo "8:0 1024" > /sys/fs/cgroup/blkio/blkio.throttle.write_bps_device [root@localhost ~]# echo $$ > /sys/fs/cgroup/blkio/cgroup.procs [root@localhost ~]# dd if=/dev/zero of=/dev/sda bs=10k count=1 oflag=direct & [1] 620 [root@localhost ~]# dd if=/dev/zero of=/dev/sda bs=10k count=1 oflag=direct & [2] 626 [root@localhost ~]# 1+0 records in 1+0 records out 10240 bytes (10 kB, 10 KiB) copied, 10.0038 s, 1.0 kB/s1+0 records in 1+0 records out 10240 bytes (10 kB, 10 KiB) copied, 9.23076 s, 1.1 kB/s -> the second bio is issued after 10s instead of 20s. This is because if some bios are already queued, current bio is queued directly and the flag 'BIO_THROTTLED' is set. And later, when former bios are dispatched, this bio will be dispatched without waiting at all, this is due to tg_with_in_bps_limit() will return 0 if the flag is set. Instead of setting the flag when bio starts throttle, delay to when throttle is done to fix the problem. Fixes: 9f5ede3c01f9 ("block: throttle split bio in case of iops limit") Signed-off-by: Yu Kuai <yukuai3@huawei.com> --- block/blk-throttle.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 447e1b8722f7..f952f2d942ff 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -811,7 +811,7 @@ static bool tg_with_in_bps_limit(struct throtl_grp *tg, struct bio *bio, unsigned int bio_size = throtl_bio_data_size(bio); /* no need to throttle if this bio's bytes have been accounted */ - if (bps_limit == U64_MAX || bio_flagged(bio, BIO_THROTTLED)) { + if (bps_limit == U64_MAX) { if (wait) *wait = 0; return true; @@ -1226,8 +1226,10 @@ static void blk_throtl_dispatch_work_fn(struct work_struct *work) spin_lock_irq(&q->queue_lock); for (rw = READ; rw <= WRITE; rw++) - while ((bio = throtl_pop_queued(&td_sq->queued[rw], NULL))) + while ((bio = throtl_pop_queued(&td_sq->queued[rw], NULL))) { + bio_set_flag(bio, BIO_THROTTLED); bio_list_add(&bio_list_on_stack, bio); + } spin_unlock_irq(&q->queue_lock); if (!bio_list_empty(&bio_list_on_stack)) { @@ -2134,7 +2136,8 @@ bool __blk_throtl_bio(struct bio *bio) } break; } - + /* this bio will be issued directly */ + bio_set_flag(bio, BIO_THROTTLED); /* within limits, let's charge and dispatch directly */ throtl_charge_bio(tg, bio); @@ -2190,7 +2193,6 @@ bool __blk_throtl_bio(struct bio *bio) out_unlock: spin_unlock_irq(&q->queue_lock); - bio_set_flag(bio, BIO_THROTTLED); #ifdef CONFIG_BLK_DEV_THROTTLING_LOW if (throttled || !td->track_bio_latency) -- 2.31.1
next reply other threads:[~2022-05-17 13:35 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-17 13:49 Yu Kuai [this message] 2022-05-17 13:49 ` [PATCH -next] blk-throttle: delay the setting of 'BIO_THROTTLED' to when throttle is done Yu Kuai 2022-05-18 4:07 ` Ming Lei 2022-05-18 4:07 ` Ming Lei 2022-05-18 6:30 ` yukuai (C) 2022-05-18 6:30 ` yukuai (C)
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220517134909.2910251-1-yukuai3@huawei.com \ --to=yukuai3@huawei.com \ --cc=axboe@kernel.dk \ --cc=cgroups@vger.kernel.org \ --cc=linux-block@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=ming.lei@redhat.com \ --cc=tj@kernel.org \ --cc=yi.zhang@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.