From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E14E8C43334 for ; Wed, 6 Jul 2022 20:07:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233847AbiGFUHp (ORCPT ); Wed, 6 Jul 2022 16:07:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52752 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231772AbiGFUHo (ORCPT ); Wed, 6 Jul 2022 16:07:44 -0400 Received: from out0.migadu.com (out0.migadu.com [IPv6:2001:41d0:2:267::]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AA89C1EEF5; Wed, 6 Jul 2022 13:07:42 -0700 (PDT) Date: Thu, 7 Jul 2022 04:08:35 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1657138056; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=fQpfWv3ooUZxpy2MTMOxGZlpcCsuZbo8crZuItPLldQ=; b=o4zkBFFyUqBN4yAvvaM0hyxq7Xeu8W4RiGF5C7DmWbn8Ixyo1D+oftzJuFDd6soyiWmfWc /ut1ktR+NKqcMWxQKt9Znl/qs7/TlzZJn7oLKv2ZwLBUci5o0dy3cW3KtbbaUOUJF3s2fe Jl+7Ttb3rm1gjDbaxQzxaVosuoiYoTs= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Tao Zhou To: Daniel Bristot de Oliveira Cc: Steven Rostedt , Wim Van Sebroeck , Guenter Roeck , Jonathan Corbet , Ingo Molnar , Thomas Gleixner , Peter Zijlstra , Will Deacon , Catalin Marinas , Marco Elver , Dmitry Vyukov , "Paul E. McKenney" , Shuah Khan , Gabriele Paoloni , Juri Lelli , Clark Williams , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-devel@vger.kernel.org, Tao Zhou Subject: Re: [PATCH V4 10/20] rv/monitor: Add the wwnr monitor skeleton created by dot2k Message-ID: References: <7d1b2238aa5c805501ae3dc80b213d06f6588cb3.1655368610.git.bristot@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7d1b2238aa5c805501ae3dc80b213d06f6588cb3.1655368610.git.bristot@kernel.org> X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: linux.dev Precedence: bulk List-ID: X-Mailing-List: linux-trace-devel@vger.kernel.org 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 > Cc: Guenter Roeck > Cc: Jonathan Corbet > Cc: Steven Rostedt > Cc: Ingo Molnar > Cc: Thomas Gleixner > Cc: Peter Zijlstra > Cc: Will Deacon > Cc: Catalin Marinas > Cc: Marco Elver > Cc: Dmitry Vyukov > Cc: "Paul E. McKenney" > Cc: Shuah Khan > Cc: Gabriele Paoloni > Cc: Juri Lelli > Cc: Clark Williams > Cc: linux-doc@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-trace-devel@vger.kernel.org > Signed-off-by: Daniel Bristot de Oliveira > --- > 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define MODULE_NAME "wwnr" > + > +/* > + * XXX: include required tracepoint headers, e.g., > + * #include > + */ > +#include > + > +/* > + * 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 >