Messages in this thread | | | Date | Fri, 3 Oct 2014 13:50:20 +0200 | From | Peter Zijlstra <> | Subject | Re: [rfcomm_run] WARNING: CPU: 1 PID: 79 at kernel/sched/core.c:7156 __might_sleep() |
| |
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); > } >
| |