Re: [PATCH v2 1/3] sched/dl: Implement cancel_dl_timer() to use in switched_from_dl()

Peter Zijlstra
2014-10-02T09:34:13Z

On Wed, Oct 01, 2014 at 01:04:22AM +0400, Kirill Tkhai wrote:
> From: Kirill Tkhai <ktkhai@parallels.com>
>
> hrtimer_try_to_cancel() may bring a suprise, its call may fail.

Well, not really a surprise that, its a _try_ operation after all.

> raw_spin_lock(&rq->lock)
> ... dl_task_timer raw_spin_lock(&rq->lock)
> ... raw_spin_lock(&rq->lock) ...
> switched_from_dl() ... ...
> hrtimer_try_to_cancel() ... ...
> switched_to_fair() ... ...
> ... ... ...
> ... ... ...
> raw_spin_unlock(&rq->lock) ... (asquired)
> ... ... ...
> ... ... ...
> do_exit() ... ...
> schedule() ... ...
> raw_spin_lock(&rq->lock) ... raw_spin_unlock(&rq->lock)
> ... ... ...
> raw_spin_unlock(&rq->lock) ... raw_spin_lock(&rq->lock)
> ... ... (asquired)
> put_task_struct() ... ...
> free_task_struct() ... ...
> ... ... raw_spin_unlock(&rq->lock)
> ... (asquired) ...
> ... ... ...
> ... Surprise!!! ...
>
> So, let's implement 100% guaranteed way to cancel the timer and let's
> be sure we are safe even in very unlikely situations.
>
> We do not create any problem with rq unlocking, because it already
> may happed below in pull_dl_task(). No problem with deadline tasks
> balancing too.

That doesn't sound right. pull_dl_task() is an entirely different
callchain than switched_from(). Now it might still be fine, but you
cannot compare it with pull_dl_task.