Messages in this thread | | | Date | Tue, 3 Mar 2020 08:48:55 -0500 | From | Tejun Heo <> | Subject | Re: [PATCH] cgroup-v1: freezer: optionally killable freezer |
| |
Hello,
On Wed, Feb 19, 2020 at 10:32:31AM -0800, Marco Ballesio wrote: > @@ -94,6 +94,18 @@ The following cgroupfs files are created by cgroup freezer. > Shows the parent-state. 0 if none of the cgroup's ancestors is > frozen; otherwise, 1. > > +* freezer.killable: Read-write > + > + When read, returns the killable state of a cgroup - "1" if frozen > + tasks will respond to fatal signals, or "0" if they won't. > + > + When written, this property sets the killable state of the cgroup. > + A value equal to "1" will switch the state of all frozen tasks in > + the cgroup to TASK_INTERRUPTIBLE (similarly to cgroup v2) and will > + make them react to fatal signals. A value of "0" will switch the > + state of frozen tasks to TASK_UNINTERRUPTIBLE and they won't respond > + to signals unless thawed or unfrozen.
As Roman said, I'm not too sure about adding a new cgroup1 freezer interface at this point. If we do this, *maybe* a mount option would be more minimal?
> diff --git a/kernel/freezer.c b/kernel/freezer.c > index dc520f01f99d..92de1bfe62cf 100644 > --- a/kernel/freezer.c > +++ b/kernel/freezer.c > @@ -42,6 +42,9 @@ bool freezing_slow_path(struct task_struct *p) > if (test_tsk_thread_flag(p, TIF_MEMDIE)) > return false; > > + if (cgroup_freezer_killable(p) && fatal_signal_pending(p)) > + return false; > + > if (pm_nosig_freezing || cgroup_freezing(p)) > return true; > > @@ -63,7 +66,12 @@ bool __refrigerator(bool check_kthr_stop) > pr_debug("%s entered refrigerator\n", current->comm); > > for (;;) { > - set_current_state(TASK_UNINTERRUPTIBLE); > + bool killable = cgroup_freezer_killable(current); > + > + if (killable) > + set_current_state(TASK_INTERRUPTIBLE); > + else > + set_current_state(TASK_UNINTERRUPTIBLE); > > spin_lock_irq(&freezer_lock); > current->flags |= PF_FROZEN; > @@ -75,6 +83,16 @@ bool __refrigerator(bool check_kthr_stop) > if (!(current->flags & PF_FROZEN)) > break; > was_frozen = true; > + > + /* > + * Now we're sure that there is no pending fatal signal. > + * Clear TIF_SIGPENDING to not get out of schedule() > + * immediately (if there is a non-fatal signal pending), and > + * put the task into sleep. > + */
and this looks really racy to me. What happens if this task gets a fatal signal here? We clear TIF_SIGPENDING and go to sleep?
> + if (killable) > + clear_thread_flag(TIF_SIGPENDING); > + > schedule(); > }
Thanks.
-- tejun
| |