From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Tom Zanussi <zanussi@kernel.org>
Subject: [for-next][PATCH 10/12] tracing: Add a backward-compatibility check for synthetic event creation
Date: Wed, 10 Feb 2021 21:09:37 -0500 [thread overview]
Message-ID: <20210211020949.960507233@goodmis.org> (raw)
In-Reply-To: 20210211020927.829775774@goodmis.org
From: Tom Zanussi <zanussi@kernel.org>
The synthetic event parsing rework now requires semicolons between
synthetic event fields. That requirement breaks existing users who
might already have used the old synthetic event command format, so
this adds an inner loop that can parse more than one field, if
present, between semicolons. For each field, parse_synth_field()
checks in which version that field was introduced, using
check_field_version(). The caller, __create_synth_event() can then use
that version information to determine whether or not to enforce the
requirement on the command as a whole.
In the future, if/when new features are added, the requirement will be
that any field/string containing the new feature must use semicolons,
and the check_field_version() check can then check for those and
enforce it. Using a version number allows this scheme to be extended
if necessary.
Link: https://lkml.kernel.org/r/74fcc500d561b40ce91c5ee94818c70c6b0c9330.1612208610.git.zanussi@kernel.org
[ zanussi: added check_field_version() comment from rostedt@goodmis.org ]
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_synth.c | 93 ++++++++++++++++++++++++-------
1 file changed, 74 insertions(+), 19 deletions(-)
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
index aace72426e99..2979a96595b4 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -580,11 +580,29 @@ static void free_synth_field(struct synth_field *field)
kfree(field);
}
-static struct synth_field *parse_synth_field(int argc, char **argv)
+static int check_field_version(const char *prefix, const char *field_type,
+ const char *field_name)
+{
+ /*
+ * For backward compatibility, the old synthetic event command
+ * format did not require semicolons, and in order to not
+ * break user space, that old format must still work. If a new
+ * feature is added, then the format that uses the new feature
+ * will be required to have semicolons, as nothing that uses
+ * the old format would be using the new, yet to be created,
+ * feature. When a new feature is added, this will detect it,
+ * and return a number greater than 1, and require the format
+ * to use semicolons.
+ */
+ return 1;
+}
+
+static struct synth_field *parse_synth_field(int argc, char **argv,
+ int *consumed, int *field_version)
{
const char *prefix = NULL, *field_type = argv[0], *field_name, *array;
- int len, consumed, ret = -ENOMEM;
struct synth_field *field;
+ int len, ret = -ENOMEM;
struct seq_buf s;
ssize_t size;
@@ -596,15 +614,10 @@ static struct synth_field *parse_synth_field(int argc, char **argv)
prefix = "unsigned ";
field_type = argv[1];
field_name = argv[2];
- consumed = 3;
+ *consumed += 3;
} else {
field_name = argv[1];
- consumed = 2;
- }
-
- if (consumed < argc) {
- synth_err(SYNTH_ERR_INVALID_FIELD, errpos(field_type));
- return ERR_PTR(-EINVAL);
+ *consumed += 2;
}
if (!field_name) {
@@ -612,6 +625,8 @@ static struct synth_field *parse_synth_field(int argc, char **argv)
return ERR_PTR(-EINVAL);
}
+ *field_version = check_field_version(prefix, field_type, field_name);
+
field = kzalloc(sizeof(*field), GFP_KERNEL);
if (!field)
return ERR_PTR(-ENOMEM);
@@ -1167,6 +1182,7 @@ static int __create_synth_event(const char *name, const char *raw_fields)
{
char **argv, *field_str, *tmp_fields, *saved_fields = NULL;
struct synth_field *field, *fields[SYNTH_FIELDS_MAX];
+ int consumed, cmd_version = 1, n_fields_this_loop;
int i, argc, n_fields = 0, ret = 0;
struct synth_event *event = NULL;
@@ -1212,21 +1228,60 @@ static int __create_synth_event(const char *name, const char *raw_fields)
if (!argc)
continue;
- field = parse_synth_field(argc, argv);
- if (IS_ERR(field)) {
- argv_free(argv);
- ret = PTR_ERR(field);
- goto err;
- }
+ n_fields_this_loop = 0;
+ consumed = 0;
+ while (argc > consumed) {
+ int field_version;
+
+ field = parse_synth_field(argc - consumed,
+ argv + consumed, &consumed,
+ &field_version);
+ if (IS_ERR(field)) {
+ argv_free(argv);
+ ret = PTR_ERR(field);
+ goto err;
+ }
- argv_free(argv);
+ /*
+ * Track the highest version of any field we
+ * found in the command.
+ */
+ if (field_version > cmd_version)
+ cmd_version = field_version;
+
+ /*
+ * Now sort out what is and isn't valid for
+ * each supported version.
+ *
+ * If we see more than 1 field per loop, it
+ * means we have multiple fields between
+ * semicolons, and that's something we no
+ * longer support in a version 2 or greater
+ * command.
+ */
+ if (cmd_version > 1 && n_fields_this_loop >= 1) {
+ synth_err(SYNTH_ERR_INVALID_CMD, errpos(field_str));
+ ret = -EINVAL;
+ goto err;
+ }
+
+ fields[n_fields++] = field;
+ if (n_fields == SYNTH_FIELDS_MAX) {
+ synth_err(SYNTH_ERR_TOO_MANY_FIELDS, 0);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ n_fields_this_loop++;
+ }
- fields[n_fields++] = field;
- if (n_fields == SYNTH_FIELDS_MAX) {
- synth_err(SYNTH_ERR_TOO_MANY_FIELDS, 0);
+ if (consumed < argc) {
+ synth_err(SYNTH_ERR_INVALID_CMD, 0);
ret = -EINVAL;
goto err;
}
+
+ argv_free(argv);
}
if (n_fields == 0) {
--
2.29.2
next prev parent reply other threads:[~2021-02-11 2:13 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-11 2:09 [for-next][PATCH 00/12] tracing: Updates for 5.12 Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 01/12] tracing: Do not create "enable" or "filter" files for ftrace event subsystem Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 02/12] tracepoints: Remove unnecessary "data_args" macro parameter Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 03/12] tracepoints: Do not punish non static call users Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 04/12] tracepoints: Code clean up Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 05/12] ftrace: Remove unused ftrace_force_update() Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 06/12] kprobes: Warn if the kprobe is reregistered Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 07/12] tracing/dynevent: Delegate parsing to create function Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 08/12] tracing: Rework synthetic event command parsing Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 09/12] tracing: Update synth command errors Steven Rostedt
2021-02-11 2:09 ` Steven Rostedt [this message]
2021-02-11 2:09 ` [for-next][PATCH 11/12] selftests/ftrace: Update synthetic event syntax errors Steven Rostedt
2021-02-11 2:09 ` [for-next][PATCH 12/12] selftests/ftrace: Add !event synthetic event syntax check Steven Rostedt
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=20210211020949.960507233@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=zanussi@kernel.org \
/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