Re: [patch 09/12] fs/timerfd: Use the new alarm/hrtimer functions

[Peter Zijlstra]

On Tue, Apr 07, 2026 at 10:54:58AM +0200, Thomas Gleixner wrote:

> +static u64 timerfd_alarm_restart(struct timerfd_ctx *ctx)
> +{
> +	u64 ticks = alarm_forward_now(&ctx->t.alarm, ctx->tintv) - 1;

(still confused on the alarm_forward_now() vs alarmtimer_start()
namespacing)

> +
> +	timerfd_alarm_start(ctx, alarm_get_expires(&ctx->t.alarm), false);
> +	return ticks;
> +}
> +
> +static void timerfd_hrtimer_start(struct timerfd_ctx *ctx, ktime_t exp,
> +				  const enum hrtimer_mode mode)
> +{
> +	/* Start the timer. If it's expired already, handle the callback. */
> +	if (!hrtimer_start_range_ns_user(&ctx->t.tmr, exp, 0, mode))
> +		__timerfd_triggered(ctx);
> +}
> +
> +static u64 timerfd_hrtimer_restart(struct timerfd_ctx *ctx)
> +{
> +	u64 ticks = hrtimer_forward_now(&ctx->t.tmr, ctx->tintv) - 1;
> +
> +	timerfd_hrtimer_start(ctx, hrtimer_get_expires(&ctx->t.tmr), HRTIMER_MODE_ABS);
> +	return ticks;
> +}

> -		if (ctx->expired && ctx->tintv) {
> -			/*
> -			 * If tintv != 0, this is a periodic timer that
> -			 * needs to be re-armed. We avoid doing it in the timer
> -			 * callback to avoid DoS attacks specifying a very
> -			 * short timer period.
> -			 */
> -			if (isalarm(ctx)) {
> -				ticks += alarm_forward_now(
> -					&ctx->t.alarm, ctx->tintv) - 1;
> -				alarm_restart(&ctx->t.alarm);
> -			} else {
> -				ticks += hrtimer_forward_now(&ctx->t.tmr,
> -							     ctx->tintv) - 1;
> -				hrtimer_restart(&ctx->t.tmr);
> -			}
> -		}
> +		ticks = ctx->ticks;
>  		ctx->expired = 0;
>  		ctx->ticks = 0;
> +
> +		/*
> +		 * If tintv != 0, this is a periodic timer that needs to be
> +		 * re-armed. We avoid doing it in the timer callback to avoid
> +		 * DoS attacks specifying a very short timer period.
> +		 */
> +		if (expired && ctx->tintv)
> +			ticks += timerfd_restart(ctx);
>  	}
>  	spin_unlock_irq(&ctx->wqh.lock);
>  	if (ticks) {
> @@ -526,18 +554,7 @@ static int do_timerfd_gettime(int ufd, s
>  	spin_lock_irq(&ctx->wqh.lock);
>  	if (ctx->expired && ctx->tintv) {
>  		ctx->expired = 0;
> -
> -		if (isalarm(ctx)) {
> -			ctx->ticks +=
> -				alarm_forward_now(
> -					&ctx->t.alarm, ctx->tintv) - 1;
> -			alarm_restart(&ctx->t.alarm);
> -		} else {
> -			ctx->ticks +=
> -				hrtimer_forward_now(&ctx->t.tmr, ctx->tintv)
> -				- 1;

(argh!)

> -			hrtimer_restart(&ctx->t.tmr);
> -		}
> +		ctx->ticks += timerfd_restart(ctx);
>  	}
>  	t->it_value = ktime_to_timespec64(timerfd_get_remaining(ctx));
>  	t->it_interval = ktime_to_timespec64(ctx->tintv);

What's with the -1 thing?

Anyway, this looks about right.