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

Peter Zijlstra
2026-04-07T10:09:24Z

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.