smp_call_function_single() runs its callback in IPI (hardirq)
context. mc146818_set_time() and mc146818_get_time() take rtc_lock
(spinlock_t), which is a sleeping lock on PREEMPT_RT, triggering
a lockdep "Invalid wait context" splat on Marvel SMP.

work_on_cpu() runs the callback in a kthread (process) context,
which can acquire sleeping locks.

Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
 arch/alpha/kernel/rtc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git ./arch/alpha/kernel/rtc.c ./arch/alpha/kernel/rtc.c
index cfdf90bc8b3f..4ad5846a1d71 100644
--- ./arch/alpha/kernel/rtc.c
+++ ./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"
 
@@ -155,11 +156,12 @@ union remote_data {
 	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 0;
 }
 
 static int
@@ -168,17 +170,18 @@ 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);
+		work_on_cpu(boot_cpuid, do_remote_read, &x);
 		return x.retval;
 	}
 	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 0;
 }
 
 static int
@@ -187,7 +190,7 @@ 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);
+		work_on_cpu(boot_cpuid, do_remote_set, &x);
 		return x.retval;
 	}
 	return alpha_rtc_set_time(NULL, tm);
-- 
2.53.0

On Fri, May 29, 2026 at 1:07 AM Matt Turner <mattst88@gmail.com> wrote:
>
> smp_call_function_single() runs its callback in IPI (hardirq)
> context. mc146818_set_time() and mc146818_get_time() take rtc_lock
> (spinlock_t), which is a sleeping lock on PREEMPT_RT, triggering
> a lockdep "Invalid wait context" splat on Marvel SMP.
>
> work_on_cpu() runs the callback in a kthread (process) context,
> which can acquire sleeping locks.
>
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Matt Turner <mattst88@gmail.com>
> ---
>  arch/alpha/kernel/rtc.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git ./arch/alpha/kernel/rtc.c ./arch/alpha/kernel/rtc.c
> index cfdf90bc8b3f..4ad5846a1d71 100644
> --- ./arch/alpha/kernel/rtc.c
> +++ ./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"
>
> @@ -155,11 +156,12 @@ union remote_data {
>         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 0;
>  }
>
>  static int
> @@ -168,17 +170,18 @@ 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);
> +               work_on_cpu(boot_cpuid, do_remote_read, &x);
>                 return x.retval;
>         }
>         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 0;
>  }
>
>  static int
> @@ -187,7 +190,7 @@ 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);
> +               work_on_cpu(boot_cpuid, do_remote_set, &x);
>                 return x.retval;
>         }
>         return alpha_rtc_set_time(NULL, tm);
> --
> 2.53.0
>


Hi Matt,

Very impressive works, thanks alot for taking the time to do this!

The overall approach makes sense for RT: smp_call_function_single()
runs the callback from the IPI path, so calling into mc146818 code that
takes rtc_lock is not valid once spinlock_t can sleep.

However, I don't think this should ignore the return value from
work_on_cpu(). work_on_cpu() returns fn(arg), so the callbacks can return
alpha_rtc_{read,set}_time() directly and remote_{read,set}_time() should
return work_on_cpu(...). That also avoids depending on x.retval if
work_on_cpu() itself fails.

Also, now that this path is intentionally process-context/sleepable, the
existing smp_processor_id() direct-call fast path deserves another look.
A task could test that it is on boot_cpuid and then migrate before the
direct alpha_rtc_*() call. If the access must be on boot_cpuid, either
always use work_on_cpu(boot_cpuid, ...) or protect the direct path
appropriately.

So I agree with the overall approach here. I wonder if we could simplify
the conversion by returning the alpha_rtc_{read,set}_time() result directly
from the work_on_cpu() callback and then returning work_on_cpu()
from remote_{read,set}_time(). Also, now that this path is intentionally
process-context/sleepable, do you think the existing smp_processor_id()
fast path is still safe against migration, or should we route the access
through work_on_cpu() unconditionally?

Regards

Magnus