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

[Tejun Heo <tj@kernel.org>]

Hello, Jeff.

On Tue, Nov 01, 2011 at 06:59:58AM -0400, Jeff Layton wrote:
> > I suppose we could look at going back to the world of complicated
> > signal handling and TASK_INTERRUPTIBLE, but that's not a trivial change
> > either. The TASK_WAKE_FREEZABLE flag you mention might make more sense
> > than doing that.

I see.

> Also, since task state and signal handling clearly isn't my forte...can
> you clarify what the main difference is in using a TASK_KILLABLE sleep
> vs. TASK_INTERRUPTIBLE with all signals but SIGKILL blocked?
>
> I know that the process state would end up different (S vs. D state).
> Is there anything else we'd need to be concerned with if we converted
> all these call sites to use such a scheme in lieu of a new
> TASK_WAKE_FREEZABLE flag or similar?

Manipulating sigmask would work in most cases but there are corner
cases (e.g. debug signals will force the mask off) and diddling with
sigmask in kernel generally isn't a good idea.

If TASK_KILLABLE is only used for killable && freezable, that probably
would be the cleanest solution but given the name and the fact that
people are in general much more aware of SIGKILL's then freezer, I
think adding such assumption is likely to cause very subtle bugs.  For
example, mem_cgroup_handle_oom() seems to assume that if it's waken
from TASK_KILLABLE either the condition it was waiting for is true or
it is dying.

If there are only a handful sites which need this type of behavior,
wouldn't something like the following work?

#define wait_event_freezekillable(wq, condition)			\
do {									\
	DEFINE_WAIT(__wait);						\
	for (;;) {							\
		prepare_to_wait(&wq, &__wait, TASK_INTERRUPTIBLE);	\
		if (condition || fatal_signal_pending(current))		\
			break;						\
		schedule();						\
		try_to_freeze();					\
	}								\
	finish_wait(&wq, &__wait);					\
} while (0)

Thanks.

-- 
tejun