On Thu, Oct 02, 2014 at 10:10:20PM +0200, Oleg Nesterov wrote:
> You know, I already thought about the patch below for other reasons, it
> can probably simplify other users of kthread_should_stop(). Because this
> way we can rely on the signal checks in schedule(). (Just in case, the
> patch is not complete, see TODO).
>
> As for rfcomm_run(), perhaps it can ise it too?
>
> set_kthread_wants_signal(true);
>
> add_wait_queue(&rfcomm_wq, &wait);
> for (;;) {
> // This is only possible if kthread_should_stop() == T
True because kthreads SIG_IGN everything, right?
> if (signal_pending(current))
> break;
>
> rfcomm_process_sessions();
> wait_woken(TASK_INTERRUPTIBLE);
> }
>
> Of course, this assumes that rfcomm_process_sessions() can't do something
> "really bad" if signal_pending() is true.
So from what I can think of, everything that does an INTERRUPTIBLE sleep
will 'malfunction' after that, right? Which might be quite a lot
actually.
> What do you think?
Interesting approach, but somewhat risky I tihnk, due to that
INTERRUPTIBLE thing.
> --- x/kernel/kthread.c
> +++ x/kernel/kthread.c
> @@ -49,6 +49,7 @@ struct kthread {
> enum KTHREAD_BITS {
> KTHREAD_IS_PER_CPU = 0,
> KTHREAD_SHOULD_STOP,
> + KTHREAD_WANTS_SIGNAL,
> KTHREAD_SHOULD_PARK,
> KTHREAD_IS_PARKED,
> };
> @@ -442,6 +443,21 @@ int kthread_park(struct task_struct *k)
> return ret;
> }
>
> +void set_kthread_wants_signal(bool on)
> +{
> + unsigned long *kflags = &to_kthread(current)->flags;
> + unsigned long irqflags;
> +
> + if (on) {
> + set_bit(KTHREAD_WANTS_SIGNAL, kflags);
> + } else {
> + spin_lock_irqsave(¤t->sighand->siglock, irqflags);
> + clear_bit(KTHREAD_WANTS_SIGNAL, kflags);
> + recalc_sigpending();
> + spin_unlock_irqrestore(¤t->sighand->siglock, irqflags);
> + }
> +}
> +
> /**
> * kthread_stop - stop a thread created by kthread_create().
> * @k: thread created by kthread_create().
> @@ -469,6 +485,9 @@ int kthread_stop(struct task_struct *k)
> if (kthread) {
> set_bit(KTHREAD_SHOULD_STOP, &kthread->flags);
> __kthread_unpark(k, kthread);
> + // TODO: this is racy, we need ->siglock.
> + if (test_bit(KTHREAD_WANTS_SIGNAL, &to_kthread(k)->flags))
> + set_tsk_thread_flag(k, TIF_SIGPENDING);
Right, but taking that should not really be a problem afaict, this is a
slow path if ever there was one.
> wake_up_process(k);
> wait_for_completion(&kthread->exited);
> }
>
--
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/
