| Date | Fri, 19 Aug 2011 17:52:22 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 01/16] freezer: fix current->state restoration race in refrigerator() |
| |
I'll try to read this series later.
Probably this doesn't matter since I didn't read the next patches, but
On 08/19, Tejun Heo wrote: > > refrigerator() saves current->state before entering frozen state and > restores it before returning using __set_current_state(); however, > this is racy,
Oh, yes. I even tried to ask for the explanation.
> set_current_state(TASK_INTERRUPTIBLE); > try_to_sleep(); > if (kthread_should_stop()) > break; > schedule();
Indeed, we can miss kthread->should_stop, and the patch fixes this case.
But please look at, say, kauditd_thread(), it does
DECLARE_WAITQUEUE(wait, current); set_current_state(TASK_INTERRUPTIBLE); add_wait_queue(&kauditd_wait, &wait);
if (!skb_queue_len(&audit_skb_queue)) { try_to_freeze(); schedule(); }
Now suppose that wake_up_interruptible(&kauditd_wait) happens, and after that refrigerator() restores TASK_INTERRUPTIBLE.
Any reason refrigerator() should try to restore? Shouldn't we simply change the rules? Yes, probably we will have to fix some users.
But it seems to me it is simply not possible to make this ->state restoration correct.
Oleg.
|