We want to limit the overall iops/bps of the device in cgroup v2,
however "io.max" or "io.min" do not exist in root cgroup currently,
which makes it impossible.
This patch enables io throttle for root cgroup:
- enable "io.max" and "io.min" in root
- don't skip root group in tg_iops_limit() and tg_bps_limit()
- don't skip root group in tg_conf_updated()
I'm not sure why this feature is disabled in the first place, is
there any problem or design constraint?
Signed-off-by: Yu Kuai <[email protected]>
---
block/blk-throttle.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 55c49015e533..fffe072de538 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -302,9 +302,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) {
@@ -332,9 +329,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) {
@@ -1430,9 +1424,8 @@ static void tg_conf_updated(struct throtl_grp *tg, bool global)
struct throtl_grp *parent_tg;
tg_update_has_rules(this_tg);
- /* ignore root/second level */
- if (!cgroup_subsys_on_dfl(io_cgrp_subsys) || !blkg->parent ||
- !blkg->parent->parent)
+ /* ignore root level */
+ if (!cgroup_subsys_on_dfl(io_cgrp_subsys) || !blkg->parent)
continue;
parent_tg = blkg_to_tg(blkg->parent);
/*
@@ -1771,7 +1764,6 @@ static struct cftype throtl_files[] = {
#ifdef CONFIG_BLK_DEV_THROTTLING_LOW
{
.name = "low",
- .flags = CFTYPE_NOT_ON_ROOT,
.seq_show = tg_print_limit,
.write = tg_set_limit,
.private = LIMIT_LOW,
@@ -1779,7 +1771,6 @@ static struct cftype throtl_files[] = {
#endif
{
.name = "max",
- .flags = CFTYPE_NOT_ON_ROOT,
.seq_show = tg_print_limit,
.write = tg_set_limit,
.private = LIMIT_MAX,
--
2.31.1
On 2021/09/09 22:08, Yu Kuai wrote:
> We want to limit the overall iops/bps of the device in cgroup v2,
> however "io.max" or "io.min" do not exist in root cgroup currently,
> which makes it impossible.
>
> This patch enables io throttle for root cgroup:
> - enable "io.max" and "io.min" in root
> - don't skip root group in tg_iops_limit() and tg_bps_limit()
> - don't skip root group in tg_conf_updated()
>
> I'm not sure why this feature is disabled in the first place, is
> there any problem or design constraint?
>
Ping...
BTW, sorry about the misspell of "io.low".
> Signed-off-by: Yu Kuai <[email protected]>
> ---
> block/blk-throttle.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index 55c49015e533..fffe072de538 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -302,9 +302,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) {
> @@ -332,9 +329,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) {
> @@ -1430,9 +1424,8 @@ static void tg_conf_updated(struct throtl_grp *tg, bool global)
> struct throtl_grp *parent_tg;
>
> tg_update_has_rules(this_tg);
> - /* ignore root/second level */
> - if (!cgroup_subsys_on_dfl(io_cgrp_subsys) || !blkg->parent ||
> - !blkg->parent->parent)
> + /* ignore root level */
> + if (!cgroup_subsys_on_dfl(io_cgrp_subsys) || !blkg->parent)
> continue;
> parent_tg = blkg_to_tg(blkg->parent);
> /*
> @@ -1771,7 +1764,6 @@ static struct cftype throtl_files[] = {
> #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
> {
> .name = "low",
> - .flags = CFTYPE_NOT_ON_ROOT,
> .seq_show = tg_print_limit,
> .write = tg_set_limit,
> .private = LIMIT_LOW,
> @@ -1779,7 +1771,6 @@ static struct cftype throtl_files[] = {
> #endif
> {
> .name = "max",
> - .flags = CFTYPE_NOT_ON_ROOT,
> .seq_show = tg_print_limit,
> .write = tg_set_limit,
> .private = LIMIT_MAX,
>
Hello Yu.
On Thu, Sep 09, 2021 at 10:08:15PM +0800, Yu Kuai <[email protected]> wrote:
> I'm not sure why this feature is disabled in the first place, is
> there any problem or design constraint?
The idea for v2 is that in the root cgroup remain only kernel threads that
provide "global" services and any user workload that should be
constrained is put into non-root cgroups. Additionally, if kernel
threads carry out work associated with a cgroup they can charge it to
the respective cgroup.
[snip]
> We want to limit the overall iops/bps of the device in cgroup v2,
Cui bono? (I mean what is the reason for throttling on the global level
when there's no other entity utiliting the residual?
<joke>Your drives are too fast?</joke>)
Michal
On Fri, Sep 17, 2021 at 10:41 AM Michal Koutný <[email protected]> wrote:
>
> Hello Yu.
>
> On Thu, Sep 09, 2021 at 10:08:15PM +0800, Yu Kuai <[email protected]> wrote:
> > I'm not sure why this feature is disabled in the first place, is
> > there any problem or design constraint?
>
> The idea for v2 is that in the root cgroup remain only kernel threads that
> provide "global" services and any user workload that should be
> constrained is put into non-root cgroups. Additionally, if kernel
> threads carry out work associated with a cgroup they can charge it to
> the respective cgroup.
>
> [snip]
> > We want to limit the overall iops/bps of the device in cgroup v2,
>
> Cui bono? (I mean what is the reason for throttling on the global level
> when there's no other entity utiliting the residual?
> <joke>Your drives are too fast?</joke>)
We'd be interested in something like this as well. (at least for
io.max). Our use case is providing remote devices which are a shared
resource. A "global" throttle like this (which is set by a local
management daemon) allows for throttling before sending network
traffic. It's also useful since we can put this throttle on a dm, so
we can enforce an aggregate throttle without needing backchannels to
coordinate multiple targets.
(This does also bring up: if this is a useful thing, would it make
sense to tie to the device, vs. requiring cgroup. We happen to use
cgroups so that requirement doesn't affect us).
Khazhy
>
> Michal
On 2021/09/18 3:58, Khazhy Kumykov wrote:
> On Fri, Sep 17, 2021 at 10:41 AM Michal Koutný <[email protected]> wrote:
>>
>> Hello Yu.
>>
>> On Thu, Sep 09, 2021 at 10:08:15PM +0800, Yu Kuai <[email protected]> wrote:
>>> I'm not sure why this feature is disabled in the first place, is
>>> there any problem or design constraint?
>>
>> The idea for v2 is that in the root cgroup remain only kernel threads that
>> provide "global" services and any user workload that should be
>> constrained is put into non-root cgroups. Additionally, if kernel
>> threads carry out work associated with a cgroup they can charge it to
>> the respective cgroup.
>>
>> [snip]
>>> We want to limit the overall iops/bps of the device in cgroup v2,
>>
>> Cui bono? (I mean what is the reason for throttling on the global level
>> when there's no other entity utiliting the residual?
>> <joke>Your drives are too fast?</joke>)
>
> We'd be interested in something like this as well. (at least for
> io.max). Our use case is providing remote devices which are a shared
> resource. A "global" throttle like this (which is set by a local
Our use case is similair to this, a host can provide several remote
devices to difierent client. If one client is under high io pressure,
other client might be affected. Thus we want to limit the overall
iops/bps from the client.
Thanks,
Kuai
> management daemon) allows for throttling before sending network
> traffic. It's also useful since we can put this throttle on a dm, so
> we can enforce an aggregate throttle without needing backchannels to
> coordinate multiple targets.
> (This does also bring up: if this is a useful thing, would it make
> sense to tie to the device, vs. requiring cgroup. We happen to use
> cgroups so that requirement doesn't affect us).
>
> Khazhy
>>
>> Michal
On Sun, Sep 19, 2021 at 06:31:38PM +0800, "yukuai (C)" <[email protected]> wrote:
> Our use case is similair to this, a host can provide several remote
> devices to difierent client. If one client is under high io pressure,
> other client might be affected. Thus we want to limit the overall
> iops/bps from the client.
I see where are you coming from now. (Perhaps I'd suggest
allocating/prioritizing the allowances on the hosting side. If simply
wrapping "everything" into a non-root cgroup is not enough.)
On 2021/09/18 3:58, Khazhy Kumykov wrote:
> (This does also bring up: if this is a useful thing, would it make
> sense to tie to the device, vs. requiring cgroup. We happen to use
> cgroups so that requirement doesn't affect us).
Good point, That's IMO a better idea, it'd be more consistent with other
resources for which there exist global (cgroup independent) kernel
constraints (e.g. threads-max sysctl, mem= cmdline, cpu hotplug) that
double the root cgroup contraint role.
OTOH, this also deepens the precedent of init NS root cgroup being
special (more special than a container's root cgroup).
My .02€,
Michal
Hello,
On Tue, Sep 21, 2021 at 03:44:14PM +0200, Michal Koutn? wrote:
> On 2021/09/18 3:58, Khazhy Kumykov wrote:
> > (This does also bring up: if this is a useful thing, would it make
> > sense to tie to the device, vs. requiring cgroup. We happen to use
> > cgroups so that requirement doesn't affect us).
>
> Good point, That's IMO a better idea, it'd be more consistent with other
> resources for which there exist global (cgroup independent) kernel
> constraints (e.g. threads-max sysctl, mem= cmdline, cpu hotplug) that
> double the root cgroup contraint role.
This is why I usually try to push root-cgroup level features outside cgroup
because it really doesn't have much to do with cgroups at the root level.
For visibility stuff, we do replicate quite a bit in the root level because
not doing so becomes too painful for users but for control I'm more
hesitant.
One side-way solution could be using iocost. It doesn't have root-level
control per-se but it does configure per-device attributes which define what
the device can and is allowed to do so that it can be used as the basis for
weighted fair distribution. Even if IO control is disabled from the root
level, it'll still modulate the device according to the parameters.
> OTOH, this also deepens the precedent of init NS root cgroup being
> special (more special than a container's root cgroup).
While it does seem like an aesthetical wrinkle, I don't think this is a
practical problem. System root being different is a given whether
aesthetically pleasing or not (not the most important but we have
CONFIG_CGROUPS after all). I don't think it'll lead anywhere good to try to
mask the differences.
Thanks.
--
tejun