Messages in this thread | | | Subject | Re: [PATCH][RFC] tracing: Enable tracepoints via module parameters | From | Rasmus Villemoes <> | Date | Wed, 21 Apr 2021 09:30:01 +0200 |
| |
On 20/04/2021 22.32, Steven Rostedt wrote: > On Tue, 20 Apr 2021 12:54:39 -0700 > Dan Williams <dan.j.williams@intel.com> wrote: > >> On Tue, Apr 20, 2021 at 5:55 AM Steven Rostedt <rostedt@goodmis.org> wrote: >>>> >>>> The dev_dbg() filter language is attractive, it's too bad >>> >>> Not sure what you mean by that. What filter language. Tracepoints do have a >>> pretty good filtering too. >>
> But you can add your own trace point, and even make it generic. That's what > bpf did for their bpf_trace_printk. You could convert dev_dbg() into a > tracepoint! > > > static __printf(2, 3) int __dev_dbg(const struct device *dev, char *fmt, ...) > { > static char buf[DEV_DEBUG_PRINTK_SIZE]; > unsigned long flags; > va_list ap; > int ret; > > raw_spin_lock_irqsave(&dev_dbg_printk_lock, flags); > va_start(ap, fmt); > ret = vsnprintf(buf, sizeof(buf), fmt, ap); > va_end(ap); > /* vsnprintf() will not append null for zero-length strings */ > if (ret == 0) > buf[0] = '\0';
Wrong. snprintf(buf, 16, "") will work just fine and cause a '\0' to be written to buf[0]. As will snprintf(buf, 16, "%s", ""), and any other case where there ends up being no characters printed.
The only case where snprintf does not guarantee nul-termination is when the _buffer size_ is 0, in which case vsnprintf obviously cannot and must not write anything at all (that's used for the "how much do I need to allocate" situation).
> > #define dev_dbg(dev, fmt, ...) \ > do { \ > if (trace_dev_dbg_printk_enabled()) \ > __dev_dbg(dev, fmt, ##__VA_ARGS__); \ > } while (0) > > Note, the "trace_dev_dbg_printk_enabled()" is a static branch, which means > it is a nop when the dev_dbg_printk tracepoint is not enabled, and is a jmp > to the __dev_dbg() logic when it is enabled. It's not a conditional branch.
dynamic_debug has been implemented in terms of static_keys for a long time. And that's a per-dev_dbg invocation static key. IIUC, the above would cause every single dev_dbg in the kernel to pass through the "grab a raw spin lock and do the snprintf" thing even when one is just interested in the dev_dbgs inside a single driver or function.
Rasmus
| |