On Thu, Jul 29, 2021 at 2:24 PM [off-list ref] wrote:

On Wed, Jul 21, 2021 at 11:56 AM Sean Paul [off-list ref] wrote:

quoted

From: Sean Paul <redacted>

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.

quoted

+#define DEBUG_PARM_DESC(dst) \
+"Enable debug output to " dst ", where each bit enables a debug category.\n" \

I will be borrowing that idea.

quoted

+
+MODULE_PARM_DESC(debug, DEBUG_PARM_DESC("syslog"));
 module_param_named(debug, __drm_debug_syslog, int, 0600);

quoted

+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..

Actually, this is more problematic than that.
this pr_debug has just a single point of >control,
whereas all those in macro expansions get individual lines in control file.

I had this problem in an earlier version of RFC patch
where I called pr_debug from within drm_dev_dbg etc.