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

[Gabriele Monaco <gmonaco@redhat.com>]

On Wed, 2025-04-23 at 08:50 +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@linutronix.de>
> ---
>  kernel/trace/rv/Kconfig                       |  1 +
>  kernel/trace/rv/Makefile                      |  1 +
>  kernel/trace/rv/monitors/pagefault/Kconfig    | 11 +++
>  .../trace/rv/monitors/pagefault/pagefault.c   | 82
> +++++++++++++++++++
>  .../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, 168 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..ff7df49871b2
> --- /dev/null
> +++ b/kernel/trace/rv/monitors/pagefault/pagefault.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/ftrace.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/rv.h>
> +#include <linux/sched/deadline.h>
> +#include <linux/sched/rt.h>
> +#include <linux/tracepoint.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_or_dl_task(task));
> +}

Mmh, you probably already considered that, so ignore my comment in that case.

I just realised this function would tell you a PI boosted task is an RT task,
is that acceptable in your model?
It's probably a configuration mistake on its own if an RT task following those
rules shares resources with non-RT tasks not following them, but if that's
something allowed, you may see this atom change more often than you'd like, not
sure if that can be something worth noting.

Perhaps you could add a comment saying that this is not a problem and why (e.g.
instead of using rt_or_dl_task_policy for the job).

What do you think?

Besides this detail, the monitor looks good to me
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>

Thanks,
Gabriele