Re: [PATCH v2 19/22] rv: Add rtapp_pagefault monitor

[Gabriele Monaco]

On Fri, 2025-04-11 at 09:37 +0200, Nam Cao wrote:
> Userspace real-time applications may have design flaws that they
> raise
> page faults in real-time threads, and thus have unexpected latencies.
> 
> Add an linear temporal logic monitor to detect this scenario.
> 
> Signed-off-by: Nam Cao <namcao@xxxxxxxxxxxxx>
> ---
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Paul Walmsley <paul.walmsley@xxxxxxxxxx>
> Cc: Palmer Dabbelt <palmer@xxxxxxxxxxx>
> Cc: Albert Ou <aou@xxxxxxxxxxxxxxxxx>
> Cc: Alexandre Ghiti <alex@xxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: H. Peter Anvin <hpa@xxxxxxxxx>
> Cc: Andy Lutomirski <luto@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: linux-riscv@xxxxxxxxxxxxxxxxxxx
> ---
>  kernel/trace/rv/Kconfig                       |  1 +
>  kernel/trace/rv/Makefile                      |  1 +
>  kernel/trace/rv/monitors/pagefault/Kconfig    | 11 +++
>  .../trace/rv/monitors/pagefault/pagefault.c   | 83
> +++++++++++++++++++
>  .../trace/rv/monitors/pagefault/pagefault.h   | 57 +++++++++++++
>  .../rv/monitors/pagefault/pagefault_trace.h   | 14 ++++
>  kernel/trace/rv/rv_trace.h                    |  1 +
>  tools/verification/models/rtapp/pagefault.ltl |  1 +
>  8 files changed, 169 insertions(+)
>  create mode 100644 kernel/trace/rv/monitors/pagefault/Kconfig
>  create mode 100644 kernel/trace/rv/monitors/pagefault/pagefault.c
>  create mode 100644 kernel/trace/rv/monitors/pagefault/pagefault.h
>  create mode 100644
> kernel/trace/rv/monitors/pagefault/pagefault_trace.h
>  create mode 100644 tools/verification/models/rtapp/pagefault.ltl
> 
> diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
> index 5c407d291661..6f86d8501e87 100644
> --- a/kernel/trace/rv/Kconfig
> +++ b/kernel/trace/rv/Kconfig
> @@ -42,6 +42,7 @@ source "kernel/trace/rv/monitors/scpd/Kconfig"
>  source "kernel/trace/rv/monitors/snep/Kconfig"
>  source "kernel/trace/rv/monitors/sncid/Kconfig"
>  source "kernel/trace/rv/monitors/rtapp/Kconfig"
> +source "kernel/trace/rv/monitors/pagefault/Kconfig"
>  # Add new monitors here
>  
>  config RV_REACTORS
> diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
> index 9b28c2419995..353ecf939d0e 100644
> --- a/kernel/trace/rv/Makefile
> +++ b/kernel/trace/rv/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_RV_MON_SCPD) += monitors/scpd/scpd.o
>  obj-$(CONFIG_RV_MON_SNEP) += monitors/snep/snep.o
>  obj-$(CONFIG_RV_MON_SNCID) += monitors/sncid/sncid.o
>  obj-$(CONFIG_RV_MON_RTAPP) += monitors/rtapp/rtapp.o
> +obj-$(CONFIG_RV_MON_PAGEFAULT) += monitors/pagefault/pagefault.o
>  # Add new monitors here
>  obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
>  obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
> diff --git a/kernel/trace/rv/monitors/pagefault/Kconfig
> b/kernel/trace/rv/monitors/pagefault/Kconfig
> new file mode 100644
> index 000000000000..b31dee208459
> --- /dev/null
> +++ b/kernel/trace/rv/monitors/pagefault/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +config RV_MON_PAGEFAULT
> +	depends on RV
> +	select RV_LTL_MONITOR
> +	depends on RV_MON_RTAPP
> +	default y
> +	select LTL_MON_EVENTS_ID
> +	bool "pagefault monitor"
> +	help
> +	  Monitor that real-time tasks do not raise page faults
> diff --git a/kernel/trace/rv/monitors/pagefault/pagefault.c
> b/kernel/trace/rv/monitors/pagefault/pagefault.c
> new file mode 100644
> index 000000000000..9f7a4cba39a1
> --- /dev/null
> +++ b/kernel/trace/rv/monitors/pagefault/pagefault.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/ftrace.h>
> +#include <linux/tracepoint.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/rv.h>
> +#include <rv/instrumentation.h>
> +
> +#define MODULE_NAME "pagefault"
> +
> +#include <rv_trace.h>
> +#include <trace/events/exceptions.h>
> +#include <monitors/rtapp/rtapp.h>
> +
> +#include "pagefault.h"
> +#include <rv/ltl_monitor.h>
> +
> +static void ltl_atoms_fetch(struct task_struct *task, struct
> ltl_monitor *mon)
> +{
> +	ltl_atom_set(mon, LTL_RT, rt_task(task));
> +}
> +
> +static void ltl_atoms_init(struct task_struct *task, struct
> ltl_monitor *mon, bool task_creation)
> +{
> +	if (task_creation)
> +		ltl_atom_set(mon, LTL_PAGEFAULT, false);
> +}
> +
> +static void handle_page_fault(void *data, unsigned long address,
> struct pt_regs *regs,
> +			      unsigned long error_code)
> +{
> +	ltl_atom_pulse(current, LTL_PAGEFAULT, true);
> +}
> +
> +static int enable_pagefault(void)
> +{
> +	int retval;
> +
> +	retval = ltl_monitor_init();
> +	if (retval)
> +		return retval;
> +
> +	rv_attach_trace_probe("pagefault", page_fault_kernel,
> handle_page_fault);
> +	rv_attach_trace_probe("pagefault", page_fault_user,
> handle_page_fault);
> +
> +	return 0;
> +}
> +
> +static void disable_pagefault(void)
> +{
> +	rv_pagefault.enabled = 0;
> +
> +	rv_detach_trace_probe("rtapp_pagefault", page_fault_kernel,
> handle_page_fault);
> +	rv_detach_trace_probe("rtapp_pagefault", page_fault_user,
> handle_page_fault);
> +
> +	ltl_monitor_destroy();
> +}
> +
> +static struct rv_monitor rv_pagefault = {
> +	.name = "pagefault",
> +	.description = "Monitor that RT tasks do not raise page
> faults",
> +	.enable = enable_pagefault,
> +	.disable = disable_pagefault,
> +};
> +
> +static int __init register_pagefault(void)
> +{
> +	rv_register_monitor(&rv_pagefault, &rv_rtapp);
> +	return 0;

Any reason why you aren't returning the error value from the monitor
registration?

Other than that, the monitor seems neat and reasonably easy to
generate.

May not be necessary in this series, but try to keep compatibility with
the userspace RV tool as well, you need to have some special case in
its tracing components because fields are not matching:
 # rv mon sleep -t
         rcuc/11-108      [011] event <CANT FIND FIELD final_state>   
(null) x (null)                   -> (null)                   Y
         rcuc/11-108      [011] event <CANT FIND FIELD final_state>   
(null) x (null)                   -> (null)                   Y
      ktimers/11-109      [011] event <CANT FIND FIELD final_state>   
(null) x (null)                   -> (null)                   Y
 
Thanks,
Gabriele