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.
> >
> > 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().
Oleg.
