On Thu, Jul 29, 2021 at 2:24 PM jim.cromie@gmail.com wrote:
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..
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.