On Tue, Aug 12, 2014 at 4:44 AM, Michal Hocko <mho...@suse.cz> wrote:
> Has it stopped working as a result of a3201227f803 (freezer: make
> freezing() test freeze conditions in effect instead of TIF_FREEZE)?
> It removes clear_freeze_flag from __thaw_task and so the OOM victim
> cannot escape from the refrigerator. But there were a lot of changes in
> that area at the time so I might be missing some subtle details.
Probably, I don't have time to dig the history.
>
>> __thaw_task() just checks if it's frozen and then wakes it up,
>> but the frozen task, after waking up, will check if freezing()
>> is still true and continue to freeze itself if so. __thaw_task()
>> can't make freezing() return false since it doesn't change any
>> of these conditions, especially cgroup_freezing().
>>
>> Fix this straightly by checking if the frozen process itself
>> has been killed by OOM killer, so that the frozen process will
>> recover itself and be killed finally.
>
> OK, this should work. I remember Tejun mentioned that he wanted frozen
> tasks to be killable but I have no idea where this went. Maybe we want
> to go with the OOM part now as it fixes a real bug.
I think he rejected the patch from Bart which makes frozen tasks killable.
>> +static bool i_should_thaw_myself(bool check_kthr_stop)
>
> should_thaw_current?
>
I have no strong preference, whatever you prefer...
>> @@ -147,12 +156,6 @@ void __thaw_task(struct task_struct *p)
>> {
>> unsigned long flags;
>>
>> - /*
>> - * Clear freezing and kick @p if FROZEN. Clearing is guaranteed to
>> - * be visible to @p as waking up implies wmb. Waking up inside
>> - * freezer_lock also prevents wakeups from leaking outside
>> - * refrigerator.
>> - */
>
> This is an unrelated change.
>
It is, because as I explained, __thaw_task() doesn't do what this comment
says therefore needs this patch. Or you prefer to make it a separated patch?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
