On 12/21, Roman Gushchin wrote:
>
> +static void cgroup_do_freeze(struct cgroup *cgrp, bool freeze)
> +{
> + struct css_task_iter it;
> + struct task_struct *task;
> +
> + lockdep_assert_held(&cgroup_mutex);
> +
> + spin_lock_irq(&css_set_lock);
> + if (freeze) {
> + cgrp->freezer.nr_tasks_to_freeze = __cgroup_task_count(cgrp);
> + set_bit(CGRP_FREEZE, &cgrp->flags);
> + } else {
> + clear_bit(CGRP_FREEZE, &cgrp->flags);
> + }
> + spin_unlock_irq(&css_set_lock);
> +
> + css_task_iter_start(&cgrp->self, 0, &it);
> + while ((task = css_task_iter_next(&it))) {
> + /*
> + * Ignore kernel threads here. Freezing cgroups containing
> + * kthreads isn't supported.
> + */
> + if (task->flags & PF_KTHREAD)
> + continue;
> + cgroup_freeze_task(task, freeze);
> + }
> + css_task_iter_end(&it);
I don't understand why this can race with exiting task. Or with SIGKILL which
kills a task before it sets current->frozen.
How can we trust nr_tasks_to_freeze at all?
Yes you added cgroup_dec_tasks_to_freeze() into cgroup_exit(). But it won't be
called if CGRP_FROZEN was not set yet, or because of "spurious" transitions
caused by cgroup_inc/dec_frozen_cnt() called by this or other tasks.
it seems that cgroup_exit() should check CGRP_FREEZE instead...
Oleg.
