On Tue, 25.03.14 01:03, WaLyong Cho (walyong....@samsung.com) wrote:
> /* Figure out which controllers we need */
>
> - if (c->cpu_accounting || c->cpu_shares != 1024)
> + if (c->cpu_accounting ||
> + c->startup_cpu_shares != 1024 ||
> + (manager_state(m) != MANAGER_STARTING && c->cpu_shares !=
> 1024))
This looks incorrect. Shouldn't it be like this:
...
if (c->cpu_accounting ||
(manager->state(m) == MANAGER_STARTING ? c->startup_cpu_shares :
c->cpu_shares) != 1024)
...
> mask |= CGROUP_CPUACCT | CGROUP_CPU;
>
> if (c->blockio_accounting ||
> - c->blockio_weight != 1000 ||
> + c->startup_blockio_weight != 1000 ||
> + (manager_state(m) != MANAGER_STARTING &&
> c->blockio_weight != 1000) ||
Similar here...
> + c = unit_get_cgroup_context(u);
> + if (manager_state(u->manager) == MANAGER_STARTING &&
> + (c->startup_cpu_shares_set || c->startup_blockio_weight_set)) {
> + r = set_put(u->manager->startup_units, u);
> + if (r < 0) {
> + if (r == -EEXIST)
> + r = 0;
> + else
> + return r;
> + }
> + }
Hmm, this should not be done when we realize things I think, but when we
load the configuration, for example in unit_load(), next to where we add
the automatic deps for things...
> + if (mode != UNIT_CHECK) {
> + c->startup_cpu_shares = ul;
> + unit_write_drop_in_private_format(u, mode,
> name, "StartupCPUShares=%lu", ul);
You need to set the boolean here, too...
> + if (mode != UNIT_CHECK) {
> + c->startup_blockio_weight = ul;
> + unit_write_drop_in_private_format(u, mode, name,
> "StartupBlockIOWeight=%lu", ul);
> + }
Same here....
>
> + while (!set_isempty(m->startup_units)) {
> + u = set_steal_first(m->startup_units);
> + cgroup_context_apply(m, unit_get_cgroup_context(u),
> unit_get_cgroup_mask(u), u->cgroup_path);
> + }
I'd really leave the set around and hence iterate through it here...
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
