On Wed, Jul 21, 2021 at 11:56 AM Sean Paul <sean@xxxxxxxxxx> wrote:
>
> From: Sean Paul <seanpaul@xxxxxxxxxxxx>
>
> This patch adds a new module parameter called drm.trace which accepts
> the same mask as drm.debug. When a debug category is enabled, log
> messages will be put in a new tracefs instance called drm for
> consumption.
>
> Using the new tracefs instance will allow distros to enable drm logging
> in production without impacting performance or spamming the system
> logs.
hi Paul,
I should have started reading here, not at patch-1
While I still think some of the _syslog name changes are extra,
drm_debug_enabled() needs it - rename goes with narrower purpose.
a couple comments below, trimming heavily.
> +#define DEBUG_PARM_DESC(dst) \
> +"Enable debug output to " dst ", where each bit enables a debug category.\n" \
I will be borrowing that idea.
> +
> +MODULE_PARM_DESC(debug, DEBUG_PARM_DESC("syslog"));
> module_param_named(debug, __drm_debug_syslog, int, 0600);
>
> +void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
> + struct va_format *vaf)
> +{
> + pr_debug("%s %pV", p->prefix, vaf);
Im not sure about prefixing this way.
dyndbg query needs to see the prefix in the format string at compile-time.
My RFC patch does the prefixing in the macro layer, when enabled.
when disabled, prefixing at runtime is fine.
This looks easy to shake out..
> + drm_trace_printf("%s %pV", p->prefix, vaf);
> +}
> +EXPORT_SYMBOL(__drm_printfn_debug_syslog_and_trace);
> +
> void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
> {
> pr_err("*ERROR* %s %pV", p->prefix, vaf);
> @@ -246,6 +278,14 @@ void drm_dev_printk(const struct device *dev, const char *level,
> struct va_format vaf;
> va_list args;
>
> + va_start(args, format);
> + vaf.fmt = format;
> + vaf.va = &args;
> + drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
> + dev ? dev_name(dev) : "",dev ? " " : "",
> + __builtin_return_address(0), &vaf);
> + va_end(args);
> +
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
here and below, you re-prepare vaf.
can you just prepare once, and print 2x to different printers ?
or is that not allowed for some reason?
maybe macros to reduce linecount ?
thanks
Jim
