Messages in this thread | | | Date | Sat, 27 Feb 2016 16:16:36 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH v5 08/20] kthread: Allow to cancel kthread work |
| |
On Fri, Feb 26, 2016 at 06:00:56PM +0100, Petr Mladek wrote: > On Fri 2016-02-26 17:25:52, Peter Zijlstra wrote: > > On Fri, Feb 26, 2016 at 04:38:18PM +0100, Petr Mladek wrote: > > > On Thu 2016-02-25 13:59:32, Peter Zijlstra wrote: > > > > On Wed, Feb 24, 2016 at 05:18:05PM +0100, Petr Mladek wrote: > > > > > @@ -770,7 +782,22 @@ void delayed_kthread_work_timer_fn(unsigned long __data) > > > > > if (WARN_ON_ONCE(!worker)) > > > > > return; > > > > > > > > > > - spin_lock(&worker->lock); > > > > > + /* > > > > > + * We might be unable to take the lock if someone is trying to > > > > > + * cancel this work and calls del_timer_sync() when this callback > > > > > + * has already been removed from the timer list. > > > > > + */ > > > > > + while (!spin_trylock(&worker->lock)) { > > > > > + /* > > > > > + * Busy wait with spin_is_locked() to avoid cache bouncing. > > > > > + * Break when canceling is set to avoid a deadlock. > > > > > + */ > > > > > + do { > > > > > + if (work->canceling) > > > > > + return; > > > > > + cpu_relax(); > > > > > + } while (spin_is_locked(&worker->lock)); > > > > > + } > > > > > /* Work must not be used with more workers, see queue_kthread_work(). */ > > > > > WARN_ON_ONCE(work->worker != worker);
> > And since you do add_timer() while holding the spinlock, this should all > > work out, no? > > Interesting idea. Yes, it should work. But is this really easier? The > try_again/relock/recheck code is not trivial either.
The trylock loop has very bad worst case behaviour, it really is best to avoid such patterns if at all possible.
| |