On Thu, Oct 02, 2014 at 02:31:50PM +0200, Peter Zijlstra wrote:
> @@ -2086,24 +2086,22 @@ static void rfcomm_kill_listener(void)
>
> static int rfcomm_run(void *unused)
> {
> + DEFINE_WAIT_FUNC(wait, woken_wake_function);
> BT_DBG("");
>
> set_user_nice(current, -10);
>
> rfcomm_add_listener(BDADDR_ANY);
>
> - while (1) {
> - set_current_state(TASK_INTERRUPTIBLE);
> -
> - if (kthread_should_stop())
> - break;
> + add_wait_queue(&rfcomm_wq, &wait);
> + while (!kthread_should_stop()) {
>
> /* Process stuff */
> rfcomm_process_sessions();
>
> - schedule();
> + wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
> }
> - __set_current_state(TASK_RUNNING);
> + remove_wait_queue(&rfcomm_wq, &wait);
>
> rfcomm_kill_listener();
>
Hmm, I think there's a problem there. If someone were to do
kthread_stop() before wait_woken() we'd not actually stop, because
wait_woken() doesn't test KTHREAD_SHOULD_STOP before calling schedule().
We can't unconditionally put a kthread_should_stop() in because
to_kthread() would explode on a !kthread. The other obvious solution is
adding a second function, something like wait_woken_or_stop(), but that
appears somewhat ugly to me.
Oleg, do you see another solution?
--
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/
