All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@kernel.org>
To: Avi Weiss <thnkslprpt@gmail.com>, jstultz@google.com
Cc: sboyd@kernel.org, mlichvar@redhat.com,
	linux-kernel@vger.kernel.org, Avi Weiss <thnkslprpt@gmail.com>
Subject: Re: [PATCH] alarmtimer: Preserve rearmed alarms during callback expiry
Date: Mon, 09 Nov 2026 21:29:33 +0100	[thread overview]
Message-ID: <875wy5vjde.ffs@fw13> (raw)
In-Reply-To: <20261101163954.767646-2-thnkslprpt@gmail.com>

On Sun, Nov 01 2026 at 18:39, Avi Weiss wrote:
> An alarm can be rearmed while its previous hrtimer callback is still
> running. alarmtimer_fired() then dequeues the newly armed alarm, so
> alarmtimer_suspend() no longer sees the wakeup even though the hrtimer
> remains queued.

This fails to explain wich alarm timer user is affected. POSIX-timers
and timerfd timers are not as they cancel the timer before re-arming it.
Which leaves the other two users of alarm_start_timer() which I did not
look at.

But if that actually happens then alarm_start_timer() is subtly broken
because of this sequence:

        alarm->node.expires = expires;
        alarmtime_enqueue(base, alarm);

Setting alarm->node.expires on a queued timer temporarily "corrupts" the
rbtree. That's "harmless" because it's serialized via base->lock and
timerqueue_del() does not care as of today, but that's all but robust
and future proof.

So this needs to be fixed as well and all of this needs to be
documented.
  
>  	scoped_guard(spinlock_irqsave, &base->lock)
> -		alarmtimer_dequeue(base, alarm);
> +		if (!hrtimer_is_queued(timer))
> +			alarmtimer_dequeue(base, alarm);

Reading this without a comment just makes me scratch my head and you
will scratch yours too three month down the road.

Thanks,

        tglx

      reply	other threads:[~2026-11-09 20:29 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-01 16:39 [PATCH] alarmtimer: Preserve rearmed alarms during callback expiry Avi Weiss
2026-11-09 20:29 ` Thomas Gleixner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=875wy5vjde.ffs@fw13 \
    --to=tglx@kernel.org \
    --cc=jstultz@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlichvar@redhat.com \
    --cc=sboyd@kernel.org \
    --cc=thnkslprpt@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.