diff mbox series

[01/11] blk-throttle: Limit whole system if root group is configured when on the default hierarchy

Message ID 20221123060401.20392-2-shikemeng@huawei.com
State New
Headers
Series A few bugfix and cleanup patches for blk-throttle |

Commit Message

Kemeng Shi Nov. 23, 2022, 6:03 a.m. UTC 
  Quoted from comment in throtl_pd_init: "If on the default hierarchy, we
switch to properly hierarchical behavior where limits on a given
throtl_grp are applied to the whole subtree rather than just the group
itself. e.g. If 16M read_bps limit is set on the root group, the whole
system can' exceed 16M for the device."
Commit b22c417c885ea9 ("blk-throttle: configure bps/iops limit for
cgroup in low limit") broke this rule and did not explain why.
Restore the ability to limit the whole system by root group.

Signed-off-by: Kemeng Shi <shikemeng@huawei.com>
---
 block/blk-throttle.c | 6 ------
 1 file changed, 6 deletions(-)

Comments

Tejun Heo Nov. 23, 2022, 5:11 p.m. UTC | #1 
On Wed, Nov 23, 2022 at 02:03:51PM +0800, Kemeng Shi wrote:
> Quoted from comment in throtl_pd_init: "If on the default hierarchy, we
> switch to properly hierarchical behavior where limits on a given
> throtl_grp are applied to the whole subtree rather than just the group
> itself. e.g. If 16M read_bps limit is set on the root group, the whole
> system can' exceed 16M for the device."

On the default hierarchy (cgroup2), the throttle interface files don't exist
in the root cgroup. In general, cgroup doesn't wanna be in the business of
restricting resources at the system level.

Thanks.
Kemeng Shi Nov. 24, 2022, 11:47 a.m. UTC | #2 
on 11/24/2022 1:11 AM, Tejun Heo wrote:
> On Wed, Nov 23, 2022 at 02:03:51PM +0800, Kemeng Shi wrote:
>> Quoted from comment in throtl_pd_init: "If on the default hierarchy, we
>> switch to properly hierarchical behavior where limits on a given
>> throtl_grp are applied to the whole subtree rather than just the group
>> itself. e.g. If 16M read_bps limit is set on the root group, the whole
>> system can' exceed 16M for the device."
> 
> On the default hierarchy (cgroup2), the throttle interface files don't exist
> in the root cgroup. In general, cgroup doesn't wanna be in the business of
> restricting resources at the system level.
Hi, Tejun. Thanks for review. If restricting is not needed anymore, the stale
comment "e.g. If 16M read_bps limit is set on the root group, the whole system
can' exceed 16M for the device." may better be remove. I will remove this
in next version.
diff mbox series

Patch

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 847721dc2b2b..96aa53e30e28 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -150,9 +150,6 @@  static uint64_t tg_bps_limit(struct throtl_grp *tg, int rw)
 	struct throtl_data *td;
 	uint64_t ret;
 
-	if (cgroup_subsys_on_dfl(io_cgrp_subsys) && !blkg->parent)
-		return U64_MAX;
-
 	td = tg->td;
 	ret = tg->bps[rw][td->limit_index];
 	if (ret == 0 && td->limit_index == LIMIT_LOW) {
@@ -180,9 +177,6 @@  static unsigned int tg_iops_limit(struct throtl_grp *tg, int rw)
 	struct throtl_data *td;
 	unsigned int ret;
 
-	if (cgroup_subsys_on_dfl(io_cgrp_subsys) && !blkg->parent)
-		return UINT_MAX;
-
 	td = tg->td;
 	ret = tg->iops[rw][td->limit_index];
 	if (ret == 0 && tg->td->limit_index == LIMIT_LOW) {