Messages in this thread | | | Date | Mon, 5 Jun 2023 12:59:54 +0200 | From | Frederic Weisbecker <> | Subject | Re: [patch v2a 01/20] posix-timers: Prevent RT livelock in itimer_delete() |
| |
Le Thu, Jun 01, 2023 at 10:16:34PM +0200, Thomas Gleixner a écrit : > itimer_delete() has a retry loop when the timer is concurrently expired. On > non-RT kernels this just spin-waits until the timer callback has completed. > On RT kernels this is a potential livelock when the exiting task preempted > the hrtimer soft interrupt.
It's not just RT but also archs supporting HAVE_POSIX_CPU_TIMERS_TASK_WORK
> > Replace spin_unlock() with an invocation of timer_wait_running() to handle > it the same way as the other retry loops in the posix timer code. > > Fixes: ec8f954a40da ("posix-timers: Use a callback for cancel synchronization on PREEMPT_RT") > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > V2: Remove bogus comments vs. posix CPU timers - Frederic > V2a: Send the real fixed up version > --- > kernel/time/posix-timers.c | 43 +++++++++++++++++++++++++++++++++++-------- > 1 file changed, 35 insertions(+), 8 deletions(-) > > --- a/kernel/time/posix-timers.c > +++ b/kernel/time/posix-timers.c > @@ -1037,27 +1037,52 @@ SYSCALL_DEFINE1(timer_delete, timer_t, t > } > > /* > - * return timer owned by the process, used by exit_itimers > + * Delete a timer if it is armed, remove it from the hash and schedule it > + * for RCU freeing. > */ > static void itimer_delete(struct k_itimer *timer) > { > -retry_delete: > - spin_lock_irq(&timer->it_lock); > + unsigned long flags; > > +retry_delete: > + /* > + * irqsave is required to make timer_wait_running() work. > + */ > + spin_lock_irqsave(&timer->it_lock, flags); > + > + /* > + * Even if the timer is not longer accessible from other tasks > + * it still might be armed and queued in the underlying timer > + * mechanism. Worse, that timer mechanism might run the expiry > + * function concurrently. > + */ > if (timer_delete_hook(timer) == TIMER_RETRY) { > - spin_unlock_irq(&timer->it_lock); > + /* > + * Timer is expired concurrently, prevent livelocks > + * and pointless spinning on RT.
Ditto.
Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
Thanks.
| |