[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <69bd53de-0948-469f-ae58-48098af1973c@akamai.com>
Date: Fri, 5 Jan 2024 17:46:41 -0500
From: Jason Baron <jbaron@...mai.com>
To: Łukasz Bartosik <lb@...ihalf.com>,
Jim Cromie <jim.cromie@...il.com>,
Andrew Morton
<akpm@...ux-foundation.org>,
Kees Cook <keescook@...omium.org>,
Douglas Anderson <dianders@...omium.org>
Cc: Guenter Roeck <groeck@...gle.com>, Yaniv Tzoreff <yanivt@...gle.com>,
Benson Leung <bleung@...gle.com>, Steven Rostedt <rostedt@...dmis.org>,
Vincent Whitchurch <vincent.whitchurch@...s.com>,
Pekka Paalanen <ppaalanen@...il.com>,
Sean Paul <seanpaul@...omium.org>, Daniel Vetter <daniel@...ll.ch>,
linux-kernel@...r.kernel.org, upstream@...ihalf.com
Subject: Re: [PATCH v2 10/15] dyndbg: add open and close commands for trace
On 11/30/23 6:40 PM, Łukasz Bartosik wrote:
> Add open and close commands for opening and closing trace instances.
> The open command has to be mandatory followed by a trace instance name.
> If a trace instance already exists in <debugfs>/tracing/instances
> directory then the open command will reuse it otherwise a new trace
> instance with a name provided to the open will be created. Close
> command closes previously opened trace instance. The close will
> fail if a user tries to close non-existent trace instances or an
> instance which was not previously opened.
>
> For example the following command will open (create or reuse existing)
> trace instance located in <debugfs>/tracing/instances/usbcore:
>
> echo "open usbcore" > <debugfs>/dynamic_debug/control
Hi,
I'm wondering why this needs to be part of dynamic debug? Can't we make
trace instances via 'mkdir' and then use those from the dynamic debug side?
Thanks,
-Jason
>
> Signed-off-by: Łukasz Bartosik <lb@...ihalf.com>
> ---
> lib/Kconfig.debug | 1 +
> lib/dynamic_debug.c | 193 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 194 insertions(+)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 5bc56c7247a2..f184c3c91ba3 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -181,6 +181,7 @@ config DYNAMIC_DEBUG_CORE
> bool "Enable core function of dynamic debug support"
> depends on PRINTK
> depends on (DEBUG_FS || PROC_FS)
> + depends on TRACING
> help
> Enable core functional support of dynamic debug. It is useful
> when you want to tie dynamic debug to your kernel modules with
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 0dc9ec76b867..43e94023a4eb 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -36,6 +36,7 @@
> #include <linux/sched.h>
> #include <linux/device.h>
> #include <linux/netdevice.h>
> +#include <linux/trace.h>
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/dyndbg.h>
> @@ -73,6 +74,25 @@ struct flag_settings {
> unsigned int mask;
> };
>
> +#define DD_OPEN_CMD "open"
> +#define DD_CLOSE_CMD "close"
> +#define DD_TR_EVENT "0"
> +
> +struct ddebug_trace_inst {
> + const char *name;
> + struct trace_array *arr;
> +};
> +
> +/*
> + * TRACE_DST_MAX value is reserved for writing
> + * debug logs to trace events (prdbg, devdbg)
> + */
> +struct ddebug_trace {
> + struct ddebug_trace_inst inst[TRACE_DST_MAX-1];
> + DECLARE_BITMAP(bmap, TRACE_DST_MAX-1);
> + int bmap_size;
> +};
> +
> static DEFINE_MUTEX(ddebug_lock);
> static LIST_HEAD(ddebug_tables);
> static int verbose;
> @@ -80,6 +100,8 @@ module_param(verbose, int, 0644);
> MODULE_PARM_DESC(verbose, " dynamic_debug/control processing "
> "( 0 = off (default), 1 = module add/rm, 2 = >control summary, 3 = parsing, 4 = per-site changes)");
>
> +static struct ddebug_trace tr = { .bmap_size = TRACE_DST_MAX-1 };
> +
> static inline struct dd_ctrl *get_ctrl(struct _ddebug *desc)
> {
> return &desc->ctrl;
> @@ -171,6 +193,148 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
> query->first_lineno, query->last_lineno, query->class_string);
> }
>
> +static bool is_ddebug_cmd(const char *str)
> +{
> + if (!strcmp(str, DD_OPEN_CMD) ||
> + !strcmp(str, DD_CLOSE_CMD))
> + return true;
> +
> + return false;
> +}
> +
> +static bool validate_tr_name(const char *str)
> +{
> + /* "0" is reserved for writing debug logs to trace events (prdbg, devdbg) */
> + if (!strcmp(str, DD_TR_EVENT))
> + return false;
> +
> + /* we allow trace instance names to include ^\w+ and underscore */
> + while (*str != '\0') {
> + if (!isalnum(*str) && *str != '_')
> + return false;
> + str++;
> + }
> +
> + return true;
> +}
> +
> +static int find_tr_instance(const char *name)
> +{
> + int idx;
> +
> + for_each_set_bit(idx, tr.bmap, tr.bmap_size)
> + if (!strcmp(tr.inst[idx].name, name))
> + return idx;
> +
> + return -ENOENT;
> +}
> +
> +static int handle_tr_opend_cmd(const char *arg)
> +{
> + struct ddebug_trace_inst *inst;
> + int idx, ret = 0;
> +
> + mutex_lock(&ddebug_lock);
> +
> + idx = find_first_zero_bit(tr.bmap, tr.bmap_size);
> + if (idx == tr.bmap_size) {
> + ret = -ENOSPC;
> + goto end;
> + }
> +
> + if (!validate_tr_name(arg)) {
> + pr_err("invalid instance name:%s\n", arg);
> + ret = -EINVAL;
> + goto end;
> + }
> +
> + if (find_tr_instance(arg) >= 0) {
> + pr_err("instance is already opened name:%s\n ", arg);
> + ret = -EEXIST;
> + goto end;
> + }
> +
> + inst = &tr.inst[idx];
> + inst->name = kstrdup(arg, GFP_KERNEL);
> + if (!inst->name) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + inst->arr = trace_array_get_by_name(inst->name);
> + if (!inst->arr) {
> + ret = -EINVAL;
> + goto end;
> + }
> +
> + ret = trace_array_init_printk(inst->arr);
> + if (ret) {
> + trace_array_put(inst->arr);
> + trace_array_destroy(inst->arr);
> + goto end;
> + }
> +
> + set_bit(idx, tr.bmap);
> + v3pr_info("opened trace instance idx=%d, name=%s\n", idx, arg);
> +end:
> + mutex_unlock(&ddebug_lock);
> + return ret;
> +}
> +
> +static int handle_tr_close_cmd(const char *arg)
> +{
> + struct ddebug_trace_inst *inst;
> + int idx, ret = 0;
> +
> + mutex_lock(&ddebug_lock);
> +
> + idx = find_tr_instance(arg);
> + if (idx < 0) {
> + ret = idx;
> + goto end;
> + }
> +
> + inst = &tr.inst[idx];
> +
> + trace_array_put(inst->arr);
> + /*
> + * don't destroy trace instance but let user do it manually
> + * with rmdir command at a convenient time later, if it is
> + * destroyed here all debug logs will be lost
> + *
> + * trace_array_destroy(inst->arr);
> + */
> + inst->arr = NULL;
> +
> + kfree(inst->name);
> + inst->name = NULL;
> +
> + clear_bit(idx, tr.bmap);
> + v3pr_info("closed trace instance idx=%d, name=%s\n", idx, arg);
> +end:
> + mutex_unlock(&ddebug_lock);
> + return ret;
> +}
> +
> +static int ddebug_parse_cmd(char *words[], int nwords)
> +{
> + int ret;
> +
> + if (nwords != 1)
> + return -EINVAL;
> +
> + if (!strcmp(words[0], DD_OPEN_CMD))
> + ret = handle_tr_opend_cmd(words[1]);
> + else if (!strcmp(words[0], DD_CLOSE_CMD))
> + ret = handle_tr_close_cmd(words[1]);
> + else {
> + pr_err("invalid command %s\n", words[0]);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
> const char *class_string, int *class_id)
> {
> @@ -567,6 +731,11 @@ static int ddebug_exec_query(char *query_string, const char *modname)
> pr_err("tokenize failed\n");
> return -EINVAL;
> }
> +
> + /* check for open, close commands */
> + if (is_ddebug_cmd(words[0]))
> + return ddebug_parse_cmd(words, nwords-1);
> +
> /* check flags 1st (last arg) so query is pairs of spec,val */
> if (ddebug_parse_flags(words[nwords-1], &modifiers)) {
> pr_err("flags parse failed\n");
> @@ -1191,6 +1360,20 @@ static struct _ddebug *ddebug_iter_next(struct ddebug_iter *iter)
> return &iter->table->ddebugs[iter->idx];
> }
>
> +/*
> + * Check if the iterator points to the last _ddebug object
> + * to traverse.
> + */
> +static bool ddebug_iter_is_last(struct ddebug_iter *iter)
> +{
> + if (iter->table == NULL)
> + return false;
> + if (iter->idx-1 < 0 &&
> + list_is_last(&iter->table->link, &ddebug_tables))
> + return true;
> + return false;
> +}
> +
> /*
> * Seq_ops start method. Called at the start of every
> * read() call from userspace. Takes the ddebug_lock and
> @@ -1281,6 +1464,16 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
> }
> seq_puts(m, "\n");
>
> + if (ddebug_iter_is_last(iter) &&
> + !bitmap_empty(tr.bmap, tr.bmap_size)) {
> + int idx;
> +
> + seq_puts(m, "\n");
> + seq_puts(m, "Opened trace instances:\n");
> + for_each_set_bit(idx, tr.bmap, tr.bmap_size)
> + seq_printf(m, "%s\n", tr.inst[idx].name);
> + }
> +
> return 0;
> }
>
Powered by blists - more mailing lists