Re: [tip:timers/core] hrtimer: Prepare support for PREEMPT_RT

[Frederic Weisbecker]

On Thu, Aug 01, 2019 at 12:04:03PM -0700, tip-bot for Anna-Maria Gleixner wrote:
> Commit-ID:  f61eff83cec9cfab31fd30a2ca8856be379cdcd5
> Gitweb:     https://git.kernel.org/tip/f61eff83cec9cfab31fd30a2ca8856be379cdcd5
> Author:     Anna-Maria Gleixner <anna-maria@xxxxxxxxxxxxx>
> AuthorDate: Fri, 26 Jul 2019 20:30:59 +0200
> Committer:  Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> CommitDate: Thu, 1 Aug 2019 20:51:22 +0200
> 
> hrtimer: Prepare support for PREEMPT_RT
> 
> When PREEMPT_RT is enabled, the soft interrupt thread can be preempted.  If
> the soft interrupt thread is preempted in the middle of a timer callback,
> then calling hrtimer_cancel() can lead to two issues:
> 
>   - If the caller is on a remote CPU then it has to spin wait for the timer
>     handler to complete. This can result in unbound priority inversion.
> 
>   - If the caller originates from the task which preempted the timer
>     handler on the same CPU, then spin waiting for the timer handler to
>     complete is never going to end.

[...]
> +/*
> + * This function is called on PREEMPT_RT kernels when the fast path
> + * deletion of a timer failed because the timer callback function was
> + * running.
> + *
> + * This prevents priority inversion, if the softirq thread on a remote CPU
> + * got preempted, and it prevents a life lock when the task which tries to
> + * delete a timer preempted the softirq thread running the timer callback
> + * function.
> + */
> +void hrtimer_cancel_wait_running(const struct hrtimer *timer)
> +{
> +	struct hrtimer_clock_base *base = timer->base;
> +
> +	if (!timer->is_soft || !base || !base->cpu_base) {
> +		cpu_relax();
> +		return;
> +	}
> +
> +	/*
> +	 * Mark the base as contended and grab the expiry lock, which is
> +	 * held by the softirq across the timer callback. Drop the lock
> +	 * immediately so the softirq can expire the next timer. In theory
> +	 * the timer could already be running again, but that's more than
> +	 * unlikely and just causes another wait loop.
> +	 */
> +	atomic_inc(&base->cpu_base->timer_waiters);
> +	spin_lock_bh(&base->cpu_base->softirq_expiry_lock);
> +	atomic_dec(&base->cpu_base->timer_waiters);
> +	spin_unlock_bh(&base->cpu_base->softirq_expiry_lock);
> +}

So, while reviewing the posix timers series, I stumbled upon timer_wait_running() which
lacked any explanation, which led me to hrtimer_cancel_wait_running() that was
a bit more helpful but still had blurry explanation.

In the end I found the approrpiate infomation in this commit changelog.
It might be helpful for future reviewers to apply this:

---