Messages in this thread | | | Date | Fri, 19 Aug 2011 18:11:42 +0200 | Subject | Re: [PATCH 01/16] freezer: fix current->state restoration race in refrigerator() | From | Tejun Heo <> |
| |
Hello, Oleg.
On Fri, Aug 19, 2011 at 5:52 PM, Oleg Nesterov <oleg@redhat.com> wrote: > 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.
Yes, it's broken, but it's not the only thing broken. There are race conditions everywhere. Even without freezer, a lot of kthread users use kthread_should_stop() incorrectly - they test them without setting sleep state and freezable kthreads are worse as the two conditions can race each other. I think kthread is just too difficult to use. We can try to salvage the situation and make kthread more friendly but I think the better solution would be just converting everyone to use workqueue or the kthread_wq wrapper. So, yeah, the whole thing is vastly broken. Let's fix things in baby steps.
Thanks.
-- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |