[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK8ByeJ1fYSVaVQz3tERzj_5QNAN4Ggx850pKcAG3vhsoWnS+w@mail.gmail.com>
Date: Fri, 10 Nov 2023 15:53:05 +0100
From: Łukasz Bartosik <lb@...ihalf.com>
To: jim.cromie@...il.com
Cc: Jason Baron <jbaron@...mai.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Kees Cook <keescook@...omium.org>,
Douglas Anderson <dianders@...omium.org>,
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 v1 11/12] dyndbg: write debug logs to trace instance
sob., 4 lis 2023 o 22:49 <jim.cromie@...il.com> napisał(a):
>
> On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@...ihalf.com> wrote:
> >
> > When trace is enabled (T flag is set) and trace_dst field is set
> > to value greater than 0 (0 is reserved for trace events) then
> > debug logs will be written to trace instance pointed by trace_dst
> > value, for example when trace_dst value is 2 then debug logs will
> > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > Given trace instance will not be initialized until debug logs are
> > requested to be written to it and afer init will persist until
> > reboot.
> >
>
> restating 00 comments -
>
> you can get rid of integer destination ids by adding a new command: open/close
>
> $> echo \
> open kms-instance \;\
> class DRM_UT_KMS -T:kms-instance # preset-dests-disable-sites \;\
> > /proc/dynamic_debug/control
>
Instead of using above command to preset destination we could preset
destination with open command. I mean last successful
open would preset destination ? What do you think ?
>
> and +T w/o dest means use existing setting, not just 0 (unless thats
> the existing setting)
>
Sounds good.
> > Signed-off-by: Łukasz Bartosik <lb@...ihalf.com>
> > ---
> > lib/Kconfig.debug | 1 +
> > lib/dynamic_debug.c | 79 ++++++++++++++++++++++++++++++++++++++++++---
> > 2 files changed, 76 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index fa307f93fa2e..9617e92c046d 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 c5cd28e74a02..541d9d522b3b 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>
> > @@ -81,6 +82,18 @@ 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)");
> >
> > +/*
> > + * When trace is enabled (T flag is set) and trace_dst field is set
> > + * to value greater than 0 (0 is reserved for trace events) then
> > + * debug logs will be written to trace instance pointed by trace_dst
> > + * value, for example when trace_dst value is 2 then debug logs will
> > + * be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > + * Given trace instance will not be initialized until debug logs are
> > + * requested to be written to it and afer init will persist until
> > + * reboot.
> > + */
> > +static struct trace_array *trace_arr[TRACE_DST_MAX];
> > +
> > static inline struct dd_ctrl *get_ctrl(struct _ddebug *desc)
> > {
> > return &desc->ctrl;
> > @@ -255,6 +268,45 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
> > return NULL;
> > }
> >
> > +static int handle_trace_dst(struct dd_ctrl *ctrl)
> > +{
> > +#define TRACE_INST_NAME_LEN 16
> > + char instance_name[TRACE_INST_NAME_LEN];
> > + struct trace_array *arr;
> > + int ret = -EINVAL;
> > +
> > + /* check if trace (T flag) is enabled */
> > + if (!(ctrl->flags & _DPRINTK_FLAGS_TRACE))
> > + return 0;
> > +
> > + /* check if trace destination are trace events */
> > + if (!ctrl->trace_dst)
> > + return 0;
> > +
> > + /* check if trace instance is already set up */
> > + if (trace_arr[ctrl->trace_dst])
> > + return 0;
> > +
> > + snprintf(instance_name, TRACE_INST_NAME_LEN,
> > + "dyndbg_inst_%u", ctrl->trace_dst);
> > + arr = trace_array_get_by_name(instance_name);
> > + if (!arr)
> > + goto err;
> > +
> > + ret = trace_array_init_printk(arr);
> > + if (ret)
> > + goto err_init;
> > +
> > + trace_arr[ctrl->trace_dst] = arr;
> > + return 0;
> > +
> > +err_init:
> > + trace_array_put(arr);
> > + trace_array_destroy(arr);
> > +err:
> > + return ret;
> > +}
> > +
> > #define __outvar /* filled by callee */
> > /*
> > * Search the tables for _ddebug's which match the given `query' and
> > @@ -338,6 +390,9 @@ static int ddebug_change(const struct ddebug_query *query,
> > nctrl.trace_dst = modifiers->trace_dst;
> > if (!memcmp(&nctrl, get_ctrl(dp), sizeof(nctrl)))
> > continue;
> > +
> > + if (handle_trace_dst(&nctrl))
> > + continue;
> > #ifdef CONFIG_JUMP_LABEL
> > if (get_flags(dp) & _DPRINTK_FLAGS_ENABLED) {
> > if (!(nctrl.flags & _DPRINTK_FLAGS_ENABLED))
> > @@ -977,8 +1032,8 @@ static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
> > static DEFINE_PER_CPU(int, ddebug_trace_reserve);
> >
> > __printf(3, 0)
> > -static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
> > - const char *fmt, va_list args)
> > +static void ddebug_trace_event(struct _ddebug *desc, const struct device *dev,
> > + const char *fmt, va_list args)
> > {
> > struct ddebug_trace_buf *buf;
> > int bufidx;
> > @@ -1010,6 +1065,15 @@ static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
> > preempt_enable_notrace();
> > }
> >
> > +__printf(2, 0)
> > +static void ddebug_trace_instance(struct _ddebug *desc, const char *fmt,
> > + va_list *args)
> > +{
> > + struct va_format vaf = { .fmt = fmt, .va = args};
> > +
> > + trace_array_printk(trace_arr[get_trace_dst(desc)], _THIS_IP_, "%pV", &vaf);
> > +}
> > +
> > __printf(2, 3)
> > static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
> > {
> > @@ -1022,7 +1086,11 @@ static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
> > * All callers include the KERN_DEBUG prefix to keep the
> > * vprintk case simple; strip it out for tracing.
> > */
> > - ddebug_trace(desc, NULL, fmt + strlen(KERN_DEBUG), args);
> > + if (get_trace_dst(desc))
> > + ddebug_trace_instance(desc, fmt, &args);
> > + else
> > + ddebug_trace_event(desc, NULL,
> > + fmt + strlen(KERN_DEBUG), args);
> > va_end(args);
> > }
> >
> > @@ -1044,7 +1112,10 @@ static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
> > va_list args;
> >
> > va_start(args, fmt);
> > - ddebug_trace(desc, dev, fmt, args);
> > + if (get_trace_dst(desc))
> > + ddebug_trace_instance(desc, fmt, &args);
> > + else
> > + ddebug_trace_event(desc, dev, fmt, args);
> > va_end(args);
> > }
> >
> > --
> > 2.42.0.869.gea05f2083d-goog
> >
Powered by blists - more mailing lists