[RESEND PATCH v6 14/14] drm/print: Add tracefs support to the drm logging helpers

[jim.cromie at gmail.com]

On Wed, Jul 21, 2021 at 11:56 AM Sean Paul <sean at poorly.run> wrote:
>
> From: Sean Paul <seanpaul at chromium.org>
>
> 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