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