* [PATCH] rv: Support systems with time64-only syscalls
@ 2025-08-04 19:45 Palmer Dabbelt
2025-08-05 6:18 ` Gabriele Monaco
0 siblings, 1 reply; 3+ messages in thread
From: Palmer Dabbelt @ 2025-08-04 19:45 UTC (permalink / raw)
To: rostedt, namcao
Cc: mhiramat, mathieu.desnoyers, Palmer Dabbelt, gmonaco,
linux-trace-kernel, linux-kernel
From: Palmer Dabbelt <palmer@dabbelt.com>
Some systems (like 32-bit RISC-V) only have the 64-bit time_t versions
of syscalls. So handle the 32-bit time_t version of those being
undefined.
Fixes: f74f8bb246cf ("rv: Add rtapp_sleep monitor")
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
---
This seems a little ugly, as it'll blow up when neither is defined. Some
#if/#error type stuff seemed uglier, though, and that's the best I could come
up with. I figure anyone without either flavor of futex call is probably deep
enough in the weeds to just figure what blows up here...
---
kernel/trace/rv/monitors/sleep/sleep.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/trace/rv/monitors/sleep/sleep.c b/kernel/trace/rv/monitors/sleep/sleep.c
index eea447b06907..c1347da69e9d 100644
--- a/kernel/trace/rv/monitors/sleep/sleep.c
+++ b/kernel/trace/rv/monitors/sleep/sleep.c
@@ -127,7 +127,9 @@ static void handle_sys_enter(void *data, struct pt_regs *regs, long id)
mon = ltl_get_monitor(current);
switch (id) {
+#ifdef __NR_clock_nanosleep
case __NR_clock_nanosleep:
+#endif
#ifdef __NR_clock_nanosleep_time64
case __NR_clock_nanosleep_time64:
#endif
@@ -138,7 +140,9 @@ static void handle_sys_enter(void *data, struct pt_regs *regs, long id)
ltl_atom_update(current, LTL_CLOCK_NANOSLEEP, true);
break;
+#ifdef __NR_futex
case __NR_futex:
+#endif
#ifdef __NR_futex_time64
case __NR_futex_time64:
#endif
--
2.50.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] rv: Support systems with time64-only syscalls
2025-08-04 19:45 [PATCH] rv: Support systems with time64-only syscalls Palmer Dabbelt
@ 2025-08-05 6:18 ` Gabriele Monaco
2025-08-05 12:12 ` Nam Cao
0 siblings, 1 reply; 3+ messages in thread
From: Gabriele Monaco @ 2025-08-05 6:18 UTC (permalink / raw)
To: Palmer Dabbelt, rostedt, namcao
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel
On Mon, 2025-08-04 at 12:45 -0700, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <palmer@dabbelt.com>
>
> Some systems (like 32-bit RISC-V) only have the 64-bit time_t
> versions of syscalls. So handle the 32-bit time_t version of those
> being undefined.
>
> Fixes: f74f8bb246cf ("rv: Add rtapp_sleep monitor")
> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> ---
> This seems a little ugly, as it'll blow up when neither is defined.
> Some #if/#error type stuff seemed uglier, though, and that's the best
> I could come up with. I figure anyone without either flavor of futex
> call is probably deep enough in the weeds to just figure what blows
> up here...
Yeah, this is getting ugly.. I wasn't fun of this ifdeffery already but
a few of them seemed acceptable, if we are really expecting any single
one of them to potentially not be available, it isn't looking good.
What about doing in the beginning of the file something like:
/*
* Define dummy syscall numbers for systems not supporting them
*/
#ifndef __NR_whatever
#define __NR_whatever -1
#endif
#ifndef __NR_some_exotic_syscall
#define __NR_some_exotic_syscall -2
#endif
The negative number would never match, we may add a mostly
insignificant overhead checking for it but we keep the function
readable. What do you think?
I'm not sure if we can get the compiler rid of it completely, but it's
probably not worth it.
What do you think?
Thanks,
Gabriele
> ---
> kernel/trace/rv/monitors/sleep/sleep.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/trace/rv/monitors/sleep/sleep.c
> b/kernel/trace/rv/monitors/sleep/sleep.c
> index eea447b06907..c1347da69e9d 100644
> --- a/kernel/trace/rv/monitors/sleep/sleep.c
> +++ b/kernel/trace/rv/monitors/sleep/sleep.c
> @@ -127,7 +127,9 @@ static void handle_sys_enter(void *data, struct
> pt_regs *regs, long id)
> mon = ltl_get_monitor(current);
>
> switch (id) {
> +#ifdef __NR_clock_nanosleep
> case __NR_clock_nanosleep:
> +#endif
> #ifdef __NR_clock_nanosleep_time64
> case __NR_clock_nanosleep_time64:
> #endif
> @@ -138,7 +140,9 @@ static void handle_sys_enter(void *data, struct
> pt_regs *regs, long id)
> ltl_atom_update(current, LTL_CLOCK_NANOSLEEP, true);
> break;
>
> +#ifdef __NR_futex
> case __NR_futex:
> +#endif
> #ifdef __NR_futex_time64
> case __NR_futex_time64:
> #endif
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] rv: Support systems with time64-only syscalls
2025-08-05 6:18 ` Gabriele Monaco
@ 2025-08-05 12:12 ` Nam Cao
0 siblings, 0 replies; 3+ messages in thread
From: Nam Cao @ 2025-08-05 12:12 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Palmer Dabbelt, rostedt, mhiramat, mathieu.desnoyers,
linux-trace-kernel, linux-kernel
On Tue, Aug 05, 2025 at 08:18:46AM +0200, Gabriele Monaco wrote:
> On Mon, 2025-08-04 at 12:45 -0700, Palmer Dabbelt wrote:
> > From: Palmer Dabbelt <palmer@dabbelt.com>
> >
> > Some systems (like 32-bit RISC-V) only have the 64-bit time_t
> > versions of syscalls. So handle the 32-bit time_t version of those
> > being undefined.
> >
> > Fixes: f74f8bb246cf ("rv: Add rtapp_sleep monitor")
> > Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> > ---
> > This seems a little ugly, as it'll blow up when neither is defined.
> > Some #if/#error type stuff seemed uglier, though, and that's the best
> > I could come up with. I figure anyone without either flavor of futex
> > call is probably deep enough in the weeds to just figure what blows
> > up here...
>
> Yeah, this is getting ugly.. I wasn't fun of this ifdeffery already but
> a few of them seemed acceptable, if we are really expecting any single
> one of them to potentially not be available, it isn't looking good.
>
> What about doing in the beginning of the file something like:
>
> /*
> * Define dummy syscall numbers for systems not supporting them
> */
>
> #ifndef __NR_whatever
> #define __NR_whatever -1
> #endif
>
> #ifndef __NR_some_exotic_syscall
> #define __NR_some_exotic_syscall -2
> #endif
>
> The negative number would never match, we may add a mostly
> insignificant overhead checking for it but we keep the function
> readable. What do you think?
That would be much better, but the patch is fine as is, so I'm okay with
merging it:
Acked-by: Nam Cao <namcao@linutronix.de>
The patch is just matching the existing ugly #ifdef style (I wonder who
even wrote that). So cleaning it up can be done in a follow-up patch.
Nam
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-05 12:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-04 19:45 [PATCH] rv: Support systems with time64-only syscalls Palmer Dabbelt
2025-08-05 6:18 ` Gabriele Monaco
2025-08-05 12:12 ` Nam Cao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).