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