On 9/17/19 9:47 PM, Oleg Nesterov wrote:
On 09/17, Tejun Heo wrote:
(cc'ing Roman and Oleg and quoting whole body)
On Tue, Sep 17, 2019 at 02:46:45PM +0800, Honglei Wang wrote:
Process who's waiting for specific sigset shouldn't be woke up
neither it is moved between different cgroups nor the cgroup it
belongs to changes the frozen state. We'd better keep it as is
and let it wait for the desired signals coming.
Following test case is one scenario which will get "Interrupted
system call" error if we wake it up in cgroup_freeze_task().
int main(int argc, char *argv[])
{
sigset_t waitset;
int signo;
sigemptyset(&waitset);
sigaddset(&waitset, SIGINT);
sigaddset(&waitset, SIGUSR1);
pthread_sigmask(SIG_BLOCK, &waitset, NULL);
for (;;) {
signo = sigwaitinfo(&waitset, NULL);
if (signo < 0)
err(1, "sigwaitinfo() failed");
if (signo == SIGUSR1)
printf("Receive SIGUSR1\n");
else
break;
}
return 0;
}
Well, I think we do not care. Userspace should handle -EINTR and restart
if necessary. And afaics this test case can equally "fail" if it races
with the system freezer, freeze_task() can be called between
set_current_state(TASK_INTERRUPTIBLE) and freezable_schedule() in
do_sigtimedwait().
OTOH, this is another indication that kernel/cgroup/freezer.c should
interact with freezer_do_not_count/freezer_count somehow.
It looks weird if a task is happily waiting for the desired signal, but
is woke up by an move from one cgroup to another (which is really not
related to any signal action from the point of userspace), doesn't it?
And same program works well in the kernel earlier than v5.2...
I knew the freeze_task() problem, but this patch is focused on the
cgroup part. Maybe we can open another loop for the system freezer or
just leave it there as is.
At this point, seems we need to be sure if it's necessary for cgroup to
fix this problem or not.
Signed-off-by: Honglei Wang <honglei.wang@xxxxxxxxxx>
---
kernel/cgroup/freezer.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/cgroup/freezer.c b/kernel/cgroup/freezer.c
index 8cf010680678..08f6abacaa75 100644
--- a/kernel/cgroup/freezer.c
+++ b/kernel/cgroup/freezer.c
@@ -162,10 +162,14 @@ static void cgroup_freeze_task(struct task_struct *task, bool freeze)
if (freeze) {
task->jobctl |= JOBCTL_TRAP_FREEZE;
- signal_wake_up(task, false);
+
+ if (sigisemptyset(&task->real_blocked))
+ signal_wake_up(task, false);
This can't really help, ->real_blocked can be empty even if this task
sleeps in sigwaitinfo().
Sorry, I'm not quit clear about the 'sleep'. sigwaitinfo() sets
->real_blocked in do_sigtimedwait() and it goes to
freezable_schedule_hrtimeout_range() soon. After the task is woke up,
->real_blocked is cleared. Seems there is no chance to set
->real_blocked to empty in this path. Do I miss something important
here? Please correct me.
Thanks in advance,
Honglei
Oleg.
