Re: [RFC PATCH] freezer: revert 27920651fe "PM / Freezer: Makefake_signal_wake_up() wake TASK_KILLABLE tasks too"

From: Oleg Nesterov
Date: Tue Nov 01 2011 - 13:55:02 EST


On 10/31, Tejun Heo wrote:
>
> Commit 27920651fe "PM / Freezer: Make fake_signal_wake_up() wake
> TASK_KILLABLE tasks too" made freezer wake up tasks in TASK_KILLABLE
> sleep too citing non-interruptible but killable sleeps in cifs and
> nfs.
>
> I don't think we can do this. We should not send spurious unsolicited
> non-interruptible wakeups. Most synchornization constructs are built
> to cope with spurious wakeups and any INTERRUPTIBLE sleep must be able
> to handle spurious wakeups but that's not true for KILLABLE sleeps -
> KILLABLE condition cannot be cancelled.

Agreed.

For example. sys_read() or page can sleep in TASK_KILLABLE assuming that
wait/down/whatever _killable can only fail if we can not return to the
usermode.

TASK_TRACED case is obviously wrong too.

> --- a/kernel/freezer.c
> +++ b/kernel/freezer.c
> @@ -67,7 +67,7 @@ static void fake_signal_wake_up(struct task_struct *p)
> unsigned long flags;
>
> spin_lock_irqsave(&p->sighand->siglock, flags);
> - signal_wake_up(p, 1);
> + signal_wake_up(p, 0);
> spin_unlock_irqrestore(&p->sighand->siglock, flags);
> }

Agreed, this looks like a bug fix to me.

Acked-by: Oleg Nesterov <oleg@xxxxxxxxxx>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/