[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YsXrw40DP2ckPHuC@geo.homenetwork>
Date: Thu, 7 Jul 2022 04:08:35 +0800
From: Tao Zhou <tao.zhou@...ux.dev>
To: Daniel Bristot de Oliveira <bristot@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Wim Van Sebroeck <wim@...ux-watchdog.org>,
Guenter Roeck <linux@...ck-us.net>,
Jonathan Corbet <corbet@....net>,
Ingo Molnar <mingo@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Marco Elver <elver@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
"Paul E. McKenney" <paulmck@...nel.org>,
Shuah Khan <skhan@...uxfoundation.org>,
Gabriele Paoloni <gpaoloni@...hat.com>,
Juri Lelli <juri.lelli@...hat.com>,
Clark Williams <williams@...hat.com>,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-trace-devel@...r.kernel.org, Tao Zhou <tao.zhou@...ux.dev>
Subject: Re: [PATCH V4 10/20] rv/monitor: Add the wwnr monitor skeleton
created by dot2k
On Thu, Jun 16, 2022 at 10:44:52AM +0200, Daniel Bristot de Oliveira wrote:
> Per task wakeup while not running (wwnr) monitor, generated by dot2k
> with this command line:
>
> $ dot2k -d wwnr.dot -t per_task
>
> The model is:
> ----- %< -----
> digraph state_automaton {
> center = true;
> size = "7,11";
> {node [shape = plaintext, style=invis, label=""] "__init_not_running"};
> {node [shape = ellipse] "not_running"};
> {node [shape = plaintext] "not_running"};
> {node [shape = plaintext] "running"};
> "__init_not_running" -> "not_running";
> "not_running" [label = "not_running", color = green3];
> "not_running" -> "not_running" [ label = "wakeup" ];
> "not_running" -> "running" [ label = "switch_in" ];
> "running" [label = "running"];
> "running" -> "not_running" [ label = "switch_out" ];
> { rank = min ;
> "__init_not_running";
> "not_running";
> }
> }
> ----- >% -----
>
> This model is broken, the reason is that a task can be running in the
> processor without being set as RUNNABLE. Think about a task about to
> sleep:
>
> 1: set_current_state(TASK_UNINTERRUPTIBLE);
> 2: schedule();
>
> And then imagine an IRQ happening in between the lines one and two,
> waking the task up. BOOM, the wakeup will happen while the task is
> running.
>
> Q: Why do we need this model, so?
> A: To test the reactors.
>
> Cc: Wim Van Sebroeck <wim@...ux-watchdog.org>
> Cc: Guenter Roeck <linux@...ck-us.net>
> Cc: Jonathan Corbet <corbet@....net>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Will Deacon <will@...nel.org>
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Marco Elver <elver@...gle.com>
> Cc: Dmitry Vyukov <dvyukov@...gle.com>
> Cc: "Paul E. McKenney" <paulmck@...nel.org>
> Cc: Shuah Khan <skhan@...uxfoundation.org>
> Cc: Gabriele Paoloni <gpaoloni@...hat.com>
> Cc: Juri Lelli <juri.lelli@...hat.com>
> Cc: Clark Williams <williams@...hat.com>
> Cc: linux-doc@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-trace-devel@...r.kernel.org
> Signed-off-by: Daniel Bristot de Oliveira <bristot@...nel.org>
> ---
> kernel/trace/rv/monitors/wwnr/wwnr.c | 115 +++++++++++++++++++++++++++
> kernel/trace/rv/monitors/wwnr/wwnr.h | 38 +++++++++
> 2 files changed, 153 insertions(+)
> create mode 100644 kernel/trace/rv/monitors/wwnr/wwnr.c
> create mode 100644 kernel/trace/rv/monitors/wwnr/wwnr.h
>
> diff --git a/kernel/trace/rv/monitors/wwnr/wwnr.c b/kernel/trace/rv/monitors/wwnr/wwnr.c
> new file mode 100644
> index 000000000000..8ba01f0f0df8
> --- /dev/null
> +++ b/kernel/trace/rv/monitors/wwnr/wwnr.c
> @@ -0,0 +1,115 @@
> +// 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>
> +#include <rv/da_monitor.h>
> +
> +#define MODULE_NAME "wwnr"
> +
> +/*
> + * XXX: include required tracepoint headers, e.g.,
> + * #include <linux/trace/events/sched.h>
> + */
> +#include <trace/events/rv.h>
> +
> +/*
> + * This is the self-generated part of the monitor. Generally, there is no need
> + * to touch this section.
> + */
> +#include "wwnr.h"
> +
> +/*
> + * Declare the deterministic automata monitor.
> + *
> + * The rv monitor reference is needed for the monitor declaration.
> + */
> +struct rv_monitor rv_wwnr;
> +DECLARE_DA_MON_PER_TASK(wwnr, char);
> +
> +/*
> + * This is the instrumentation part of the monitor.
> + *
> + * This is the section where manual work is required. Here the kernel events
> + * are translated into model's event.
> + *
> + */
> +static void handle_switch_in(void *data, /* XXX: fill header */)
> +{
> + struct task_struct *p = /* XXX: how do I get p? */;
> + da_handle_event_wwnr(p, switch_in_wwnr);
> +}
> +
> +static void handle_switch_out(void *data, /* XXX: fill header */)
> +{
> + struct task_struct *p = /* XXX: how do I get p? */;
> + da_handle_event_wwnr(p, switch_out_wwnr);
> +}
> +
> +static void handle_wakeup(void *data, /* XXX: fill header */)
> +{
> + struct task_struct *p = /* XXX: how do I get p? */;
> + da_handle_event_wwnr(p, wakeup_wwnr);
> +}
> +
> +static int start_wwnr(void)
> +{
> + int retval;
> +
> + retval = da_monitor_init_wwnr();
> + if (retval)
> + return retval;
> +
> + rv_attach_trace_probe("wwnr", /* XXX: tracepoint */, handle_switch_in);
> + rv_attach_trace_probe("wwnr", /* XXX: tracepoint */, handle_switch_out);
> + rv_attach_trace_probe("wwnr", /* XXX: tracepoint */, handle_wakeup);
> +
> + return 0;
> +}
> +
> +static void stop_wwnr(void)
> +{
> + rv_wwnr.enabled = 0;
> +
> + rv_detach_trace_probe("wwnr", /* XXX: tracepoint */, handle_switch_in);
> + rv_detach_trace_probe("wwnr", /* XXX: tracepoint */, handle_switch_out);
> + rv_detach_trace_probe("wwnr", /* XXX: tracepoint */, handle_wakeup);
> +
> + da_monitor_destroy_wwnr();
> +}
> +
> +/*
> + * This is the monitor register section.
> + */
> +struct rv_monitor rv_wwnr = {
> + .name = "wwnr",
> + .description = "auto-generated wwnr",
> + .start = start_wwnr,
> + .stop = stop_wwnr,
> + .reset = da_monitor_reset_all_wwnr,
> + .enabled = 0,
> +};
> +
> +int register_wwnr(void)
> +{
> + rv_register_monitor(&rv_wwnr);
> + return 0;
> +}
> +
> +void unregister_wwnr(void)
> +{
> + if (rv_wwnr.enabled)
> + stop_wwnr();
> +
> + rv_unregister_monitor(&rv_wwnr);
> +}
> +
> +module_init(register_wwnr);
> +module_exit(unregister_wwnr);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("dot2k: auto-generated");
> +MODULE_DESCRIPTION("wwnr");
> diff --git a/kernel/trace/rv/monitors/wwnr/wwnr.h b/kernel/trace/rv/monitors/wwnr/wwnr.h
> new file mode 100644
> index 000000000000..f3dc160642bf
> --- /dev/null
> +++ b/kernel/trace/rv/monitors/wwnr/wwnr.h
> @@ -0,0 +1,38 @@
> +enum states_wwnr {
> + not_running_wwnr = 0,
> + running_wwnr,
> + state_max_wwnr
> +};
> +
> +enum events_wwnr {
> + switch_in_wwnr = 0,
> + switch_out_wwnr,
> + wakeup_wwnr,
> + event_max_wwnr
> +};
> +
> +struct automaton_wwnr {
> + char *state_names[state_max_wwnr];
> + char *event_names[event_max_wwnr];
> + char function[state_max_wwnr][event_max_wwnr];
> + char initial_state;
> + char final_states[state_max_wwnr];
> +};
> +
> +struct automaton_wwnr automaton_wwnr = {
> + .state_names = {
> + "not_running",
> + "running"
> + },
> + .event_names = {
> + "switch_in",
> + "switch_out",
> + "wakeup"
> + },
The .state_names and .event_names lack the wwnr postfix.
If they generated aotomatically, something need to fix,
if not, can manually change to the consistent name.
If the state change from running to non-running(initial state),
in the next patch in handle_switch() where there checked
TASK_INTERRUPTABLE to set initial state. Why can not check
TASK_UNINTERURPTABLE to set initial state there, confused.
This model is not completed for me now.
> + .function = {
> + { running_wwnr, -1, not_running_wwnr },
> + { -1, not_running_wwnr, -1 },
> + },
> + .initial_state = not_running_wwnr,
> + .final_states = { 1, 0 },
> +};
> --
> 2.35.1
>
Powered by blists - more mailing lists