[systemd-devel] [PATCH 2/2] core: add startup resource control option

[Lennart Poettering]

On Wed, 12.03.14 19:13, WaLyong Cho (walyong.cho at samsung.com) wrote:

> Similar to CPUShares= and BlockIOWeight= respectively. However only
> assign the specified weight during startup. Each control group
> attribute is re-assigned as weight by CPUShares=weight and
> BlockIOWeight=weight after startup.  If not CPUShares= or
> BlockIOWeight= be specified, then the attribute is re-assigned to each
> default attribute value. (default cpu.shares=1024, blkio.weight=1000)
> If only CPUShares=weight or BlockIOWeight=weight be specified, then
> that implies StartupCPUShares=weight and StartupBlockIOWeight=weight.

Looks pretty OK already. Could you please rework this on top of the new
manager_state() call I added? A few comments:

> --- a/src/core/cgroup.c
> +++ b/src/core/cgroup.c
> @@ -34,8 +34,12 @@ void cgroup_context_init(CGroupContext *c) {
>           * structure is preinitialized to 0 */
>  
>          c->cpu_shares = 1024;
> +        c->startup_cpu_shares = 1024;
> +        c->use_startup_cpu_shares = false;

Hmm, could we name this startup_cpu_shares_set? Similar fields in the
ExecContext use the _set suffix to indicate whether they are set. 

>          c->memory_limit = (uint64_t) -1;
>          c->blockio_weight = 1000;
> +        c->startup_blockio_weight = 1000;
> +        c->use_startup_blockio_weight = false;

Same here...

>          /* Figure out which controllers we need */
>  
> -        if (c->cpu_accounting || c->cpu_shares != 1024)
> +        if (c->cpu_accounting ||
> +            c->cpu_shares != 1024 ||
> +            c->startup_cpu_shares != 1024)
>                  mask |= CGROUP_CPUACCT | CGROUP_CPU;

This should also take the current system state into account, and only
add add the cpu controller to the mask if we are in the right state.

>  
>          if (c->blockio_accounting ||
>              c->blockio_weight != 1000 ||
> +            c->startup_blockio_weight != 1000 ||
>              c->blockio_device_weights ||
>              c->blockio_device_bandwidths)
>                  mask |= CGROUP_BLKIO;

Similar here...

There's a bit of complexity still missing: we need to refresh the cgroup
tree as soon as the startup is completed, so that the weight/shares
passed into the kernel are corrected. For this, we probably need to add
a "Set" object to the Manager object that contains all units that
require refreshing like this. Then, whenever we load a unit that sets
the startup weight/shares, it should be added to this set. Finally, in
manager_check_finished() add some code that iterates through this set,
and readds it to the cgroup queue.

Lennart

-- 
Lennart Poettering, Red Hat