lkml.org 
[lkml]   [2014]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [Patch v2] freezer: check OOM kill while being frozen
From
On Tue, Aug 12, 2014 at 4:44 AM, Michal Hocko <mhocko@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.


\
 
 \ /
  Last update: 2014-08-13 03:01    [W:0.067 / U:0.880 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site