Re: [PATCH 1/2] alpha: run the remote RTC access in a worker, not an IPI callback

[Magnus Lindholm]

On Mon, Aug 10, 2026 at 10:28 PM Matt Turner <mattst88@xxxxxxxxx> wrote:
>
> On Marvel the CMOS clock is only reachable from the boot cpu, so
> remote_read_time() and remote_set_time() bounce the access there with
> smp_call_function_single(), whose callback runs in hard interrupt
> context.
>
> alpha_rtc_read_time() calls mc146818_get_time() with a 10 ms timeout.
> That waits out the RTC update cycle in mc146818_avoid_UIP(), which drops
> rtc_lock and udelay()s 100 us at a time until the update completes or
> the timeout expires:
>
>         for (i = 0; UIP_RECHECK_LOOPS_MS(i) < timeout; i++) {
>                 spin_lock_irqsave(&rtc_lock, flags);
>                 ...
>                 if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) {
>                         spin_unlock_irqrestore(&rtc_lock, flags);
>                         udelay(UIP_RECHECK_DELAY);
>                         continue;
>                 }
>
> So a clock read from a non-boot cpu can spin for up to 10 ms in hard
> interrupt context on the boot cpu, while the cpu that sent the request
> spins in smp_call_function_single() waiting for it to finish.
>
> mc146818_set_time() does not poll, but it takes rtc_lock too, and
> rtc_lock is a spinlock_t.  Only raw spinlocks may be taken in hard
> interrupt context, so lockdep reports the write path as soon as a
> non-boot cpu sets the clock:
>
>   [ BUG: Invalid wait context ]
>   -----------------------------
>   swapper/0/0 is trying to lock:
>   fffffc0003690470 (rtc_lock){....}-{3:3}, at: mc146818_set_time+0x74/0x450
>   other info that might help us debug this:
>   context-{2:2}
>   no locks held by swapper/0/0.
>   stack backtrace:
>   CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 7.2.0-rc1 #1 NONE
>   Trace:
>   [<fffffc000102ebb0>] dump_stack+0x28/0x44
>   [<fffffc000110efcc>] __lock_acquire+0xb0c/0x1060
>   [<fffffc000110f5f0>] lock_acquire.part.0+0xd0/0x300
>   [...]
>   [<fffffc0001b0d834>] mc146818_set_time+0x74/0x450
>   [<fffffc0001f090cc>] _raw_spin_lock_irqsave+0x7c/0xc0
>   [<fffffc0001042f90>] do_remote_set+0x90/0xc0
>   [<fffffc000119c1a4>] __flush_smp_call_function_queue+0x314/0x5c0
>   [<fffffc000119c474>] generic_smp_call_function_single_interrupt+0x24/0x40
>   [<fffffc000103d984>] handle_ipi+0xa4/0x230
>   [<fffffc0001037044>] do_entInt+0x1a4/0x2e0
>
> The rtc class ops are always called from process context, so there is no
> reason to run the access from an interrupt at all.  Use work_on_cpu() to
> run it in a worker on the boot cpu.  Alpha does not support cpu hotplug,
> so the boot cpu cannot go offline while the work is pending.
>
> Tested on an AlphaServer ES47 (Marvel/EV7): hwclock read and write
> pinned to a non-boot cpu, twenty times, with no splat.
>
> Signed-off-by: Matt Turner <mattst88@xxxxxxxxx>
> ---
>  arch/alpha/kernel/rtc.c | 37 ++++++++++++-------------------------
>  1 file changed, 12 insertions(+), 25 deletions(-)
>
> diff --git a/arch/alpha/kernel/rtc.c b/arch/alpha/kernel/rtc.c
> index cfdf90bc8b3f..9e7d714ef6f8 100644
> --- a/arch/alpha/kernel/rtc.c
> +++ b/arch/alpha/kernel/rtc.c
> @@ -15,6 +15,7 @@
>  #include <linux/bcd.h>
>  #include <linux/rtc.h>
>  #include <linux/platform_device.h>
> +#include <linux/workqueue.h>
>
>  #include "proto.h"
>
> @@ -142,54 +143,40 @@ static const struct rtc_class_ops alpha_rtc_ops = {
>  };
>
>  /*
> - * Similarly, except do the actual CMOS access on the boot cpu only.
> - * This requires marshalling the data across an interprocessor call.
> + * Similarly, except do the actual CMOS access on the boot cpu only.  The
> + * access polls for the RTC update cycle and takes rtc_lock, so run it in a
> + * worker on that cpu rather than from an interprocessor interrupt.
>   */
>
>  #if defined(CONFIG_SMP) && \
>      (defined(CONFIG_ALPHA_GENERIC) || defined(CONFIG_ALPHA_MARVEL))
>  # define HAVE_REMOTE_RTC 1
>
> -union remote_data {
> -       struct rtc_time *tm;
> -       long retval;
> -};
> -
> -static void
> +static long
>  do_remote_read(void *data)
>  {
> -       union remote_data *x = data;
> -       x->retval = alpha_rtc_read_time(NULL, x->tm);
> +       return alpha_rtc_read_time(NULL, data);
>  }
>
>  static int
>  remote_read_time(struct device *dev, struct rtc_time *tm)
>  {
> -       union remote_data x;
> -       if (smp_processor_id() != boot_cpuid) {
> -               x.tm = tm;
> -               smp_call_function_single(boot_cpuid, do_remote_read, &x, 1);
> -               return x.retval;
> -       }
> +       if (smp_processor_id() != boot_cpuid)
> +               return work_on_cpu(boot_cpuid, do_remote_read, tm);
>         return alpha_rtc_read_time(NULL, tm);
>  }
>
> -static void
> +static long
>  do_remote_set(void *data)
>  {
> -       union remote_data *x = data;
> -       x->retval = alpha_rtc_set_time(NULL, x->tm);
> +       return alpha_rtc_set_time(NULL, data);
>  }
>
>  static int
>  remote_set_time(struct device *dev, struct rtc_time *tm)
>  {
> -       union remote_data x;
> -       if (smp_processor_id() != boot_cpuid) {
> -               x.tm = tm;
> -               smp_call_function_single(boot_cpuid, do_remote_set, &x, 1);
> -               return x.retval;
> -       }
> +       if (smp_processor_id() != boot_cpuid)
> +               return work_on_cpu(boot_cpuid, do_remote_set, tm);
>         return alpha_rtc_set_time(NULL, tm);
>  }
>
Hi Matt,

The change looks good to me. I also built and booted the series on
an AlphaServer ES40, although that machine does not exercise the
Marvel remote-RTC path.

Reviewed-by: Magnus Lindholm <linmag7@xxxxxxxxx>

Thanks,
Magnus