Linux-Trace-Devel Archive mirror
 help / color / mirror / Atom feed
From: Ze Gao <zegao2021@gmail.com>
To: Adrian Hunter <adrian.hunter@intel.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ian Rogers <irogers@google.com>, Ingo Molnar <mingo@redhat.com>,
	Jiri Olsa <jolsa@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org,
	linux-trace-devel@vger.kernel.org, Ze Gao <zegao@tencent.com>
Subject: [RFC PATCH v3 0/6] add to report task state in symbolic chars from sched tracepoint
Date: Tue,  1 Aug 2023 17:01:18 +0800	[thread overview]
Message-ID: <20230801090124.8050-1-zegao@tencent.com> (raw)

This is the 3rd attempt to fix the report task state issue in sched
tracepint, you can check out previous discussions here:

v1: https://lore.kernel.org/linux-trace-kernel/20230725072254.32045-1-zegao@tencent.com
v2: https://lore.kernel.org/linux-trace-kernel/20230726121618.19198-1-zegao@tencent.com

FYI, this series are designed not to break anything now and still do the 
1-1 correspondence int-char mapping for each distinct task state we want to
report, and thus will not lose any details intended for debug purposes. Of
course, this might be compromised because of bugs introduced due to my
stupidity. So your sage comments are very important and appreciated!


diff from v2:
1. reorder to condense sched_switch record structure
2. fallback to older method to maintain backward compatibility
   for perf/libtraceevent
3. split housekeeping work into separate ones for readability

--

In the status quo, we should see three different outcomes of the reported
sched-out task state from perf-script, perf-sched-timehist, and Tp_printk
of tracepoint sched_switch.  And it's not hard to figure out that the
former two are built upon the third one, and the reason why we see this
inconsistency is that the former two does not catch up with the internal
change of reported task state definitions as the kernel evolves.

IMHO, exporting internal representations of task state in the tracepoint
sched_switch is not a good practice and not encouraged at all, which can
easily break userspace tools that relies on it. Especially when tracepoints
are massively used in many observability tools nowadays due to its stable
nature, which makes them no longer used for debug only purpose and we
should be careful to decide what ought to be reported to userspace and what
ought not.

Therefore, to fix the issues mentioned above for good, I proposed to add
a new variable to report task state in sched_switch with a symbolic char
along with the old hardcoded value, and save the further processing of
userspace tools and spare them from knowing implementation details in the
kernel.

After this patch seires, we report 'RSDTtXZPI' the same as in procfs, plus
a 'p' which denotes PREEMP_ACTIVE and is used for sched_switch tracepoint
only.

Reviews welcome!

Regards,
Ze

Ze Gao (5):
  perf sched: sync state char array with the kernel
  perf sched: reorganize sched-out task state report code
  sched, tracing: add to report task state in symbolic chars
  sched, tracing: reorganize fields of switch event struct
  perf sched: prefer to use prev_state_char introduced in sched_switch

 include/trace/events/sched.h | 68 +++++++++++++++++-------------
 tools/perf/builtin-sched.c   | 82 ++++++++++++++++--------------------
 2 files changed, 76 insertions(+), 74 deletions(-)

Ze Gao (1):
  libtraceevent: prefer to use prev_state_char introduced in
    sched_switch

 plugins/plugin_sched_switch.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
2.40.1


             reply	other threads:[~2023-08-01  9:01 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-01  9:01 Ze Gao [this message]
2023-08-01  9:01 ` [RFC PATCH v3 1/6] perf sched: sync state char array with the kernel Ze Gao
2023-08-01  9:01 ` [RFC PATCH v3 2/6] perf sched: reorganize sched-out task state report code Ze Gao
2023-08-01  9:01 ` [RFC PATCH v3 3/6] sched, tracing: add to report task state in symbolic chars Ze Gao
2023-08-01 11:34   ` Peter Zijlstra
2023-08-01 13:03     ` Ze Gao
2023-08-01 13:41       ` Peter Zijlstra
2023-08-02  3:03         ` Ze Gao
2023-08-01 11:45   ` Peter Zijlstra
2023-08-01 13:08     ` Ze Gao
2023-08-01 14:27     ` Steven Rostedt
2023-08-01  9:01 ` [RFC PATCH v3 4/6] sched, tracing: reorganize fields of switch event struct Ze Gao
2023-08-01 11:46   ` Peter Zijlstra
2023-08-01 13:16     ` Ze Gao
2023-08-01 14:33     ` Steven Rostedt
2023-08-02  3:06       ` Ze Gao
2023-08-01 14:16   ` Steven Rostedt
2023-08-02  3:07     ` Ze Gao
2023-08-01  9:01 ` [RFC PATCH v3 5/6] perf sched: prefer to use prev_state_char introduced in sched_switch Ze Gao
2023-08-01  9:01 ` [RFC PATCH v3 6/6] libtraceevent: " Ze Gao
2023-08-01 14:19   ` Steven Rostedt
2023-08-02  3:08     ` Ze Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230801090124.8050-1-zegao@tencent.com \
    --to=zegao2021@gmail.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=zegao@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).