Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

posix-cpu-timers: Recalc next expiration when timer_settime() ends up not queueing

There are several scenarios that can result in posix_cpu_timer_set()
not queueing the timer but still leaving the threadgroup cputime counter
running or keeping the tick dependency around for a random amount of time.

1) If timer_settime() is called with a 0 expiration on a timer that is
already disabled, the process wide cputime counter will be started
and won't ever get a chance to be stopped by stop_process_timer()
since no timer is actually armed to be processed.

The following snippet is enough to trigger the issue.

void trigger_process_counter(void)
{
timer_t id;
struct itimerspec val = { };

timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id);
timer_settime(id, TIMER_ABSTIME, &val, NULL);
timer_delete(id);
}

2) If timer_settime() is called with a 0 expiration on a timer that is
already armed, the timer is dequeued but not really disarmed. So the
process wide cputime counter and the tick dependency may still remain
a while around.

The following code snippet keeps this overhead around for one week after
the timer deletion:

void trigger_process_counter(void)
{
timer_t id;
struct itimerspec val = { };

val.it_value.tv_sec = 604800;
timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id);
timer_settime(id, 0, &val, NULL);
timer_delete(id);
}

3) If the timer was initially deactivated, this call to timer_settime()
with an early expiration may have started the process wide cputime
counter even though the timer hasn't been queued and armed because it
has fired early and inline within posix_cpu_timer_set() itself. As a
result the process wide cputime counter may never stop until a new
timer is ever armed in the future.

The following code snippet can reproduce this:

void trigger_process_counter(void)
{
timer_t id;
struct itimerspec val = { };

signal(SIGALRM, SIG_IGN);
timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id);
val.it_value.tv_nsec = 1;
timer_settime(id, TIMER_ABSTIME, &val, NULL);
}

4) If the timer was initially armed with a former expiration value
before this call to timer_settime() and the current call sets an
early deadline that has already expired, the timer fires inline
within posix_cpu_timer_set(). In this case it must have been dequeued
before firing inline with its new expiration value, yet it hasn't
been disarmed in this case. So the process wide cputime counter and
the tick dependency may still be around for a while even after the
timer fired.

The following code snippet can reproduce this:

void trigger_process_counter(void)
{
timer_t id;
struct itimerspec val = { };

signal(SIGALRM, SIG_IGN);
timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id);
val.it_value.tv_sec = 100;
timer_settime(id, TIMER_ABSTIME, &val, NULL);
val.it_value.tv_sec = 0;
val.it_value.tv_nsec = 1;
timer_settime(id, TIMER_ABSTIME, &val, NULL);
}

Fix all these issues with triggering the related base next expiration
recalculation on the next tick. This also implies to re-evaluate the need
to keep around the process wide cputime counter and the tick dependency, in
a similar fashion to disarm_timer().

Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20210726125513.271824-7-frederic@kernel.org

authored by

Frederic Weisbecker and committed by
Thomas Gleixner
ee375328 5c8f23e6

+41 -7
+6 -1
include/linux/posix-timers.h
··· 82 82 return timerqueue_add(head, &ctmr->node); 83 83 } 84 84 85 + static inline bool cpu_timer_queued(struct cpu_timer *ctmr) 86 + { 87 + return !!ctmr->head; 88 + } 89 + 85 90 static inline bool cpu_timer_dequeue(struct cpu_timer *ctmr) 86 91 { 87 - if (ctmr->head) { 92 + if (cpu_timer_queued(ctmr)) { 88 93 timerqueue_del(ctmr->head, &ctmr->node); 89 94 ctmr->head = NULL; 90 95 return true;
+35 -6
kernel/time/posix-cpu-timers.c
··· 419 419 } 420 420 421 421 /* 422 + * Force recalculating the base earliest expiration on the next tick. 423 + * This will also re-evaluate the need to keep around the process wide 424 + * cputime counter and tick dependency and eventually shut these down 425 + * if necessary. 426 + */ 427 + static void trigger_base_recalc_expires(struct k_itimer *timer, 428 + struct task_struct *tsk) 429 + { 430 + struct posix_cputimer_base *base = timer_base(timer, tsk); 431 + 432 + base->nextevt = 0; 433 + } 434 + 435 + /* 422 436 * Dequeue the timer and reset the base if it was its earliest expiration. 423 437 * It makes sure the next tick recalculates the base next expiration so we 424 438 * don't keep the costly process wide cputime counter around for a random ··· 452 438 453 439 base = timer_base(timer, p); 454 440 if (cpu_timer_getexpires(ctmr) == base->nextevt) 455 - base->nextevt = 0; 441 + trigger_base_recalc_expires(timer, p); 456 442 } 457 443 458 444 ··· 748 734 timer->it_overrun_last = 0; 749 735 timer->it_overrun = -1; 750 736 751 - if (new_expires != 0 && !(val < new_expires)) { 737 + if (val >= new_expires) { 738 + if (new_expires != 0) { 739 + /* 740 + * The designated time already passed, so we notify 741 + * immediately, even if the thread never runs to 742 + * accumulate more time on this clock. 743 + */ 744 + cpu_timer_fire(timer); 745 + } 746 + 752 747 /* 753 - * The designated time already passed, so we notify 754 - * immediately, even if the thread never runs to 755 - * accumulate more time on this clock. 748 + * Make sure we don't keep around the process wide cputime 749 + * counter or the tick dependency if they are not necessary. 756 750 */ 757 - cpu_timer_fire(timer); 751 + sighand = lock_task_sighand(p, &flags); 752 + if (!sighand) 753 + goto out; 754 + 755 + if (!cpu_timer_queued(ctmr)) 756 + trigger_base_recalc_expires(timer, p); 757 + 758 + unlock_task_sighand(p, &flags); 758 759 } 759 760 out: 760 761 rcu_read_unlock();