| Message ID | 20250320185238.447458-14-jim.cromie@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y | expand |
Le 20/03/2025 à 19:51, Jim Cromie a écrit : > Add _INCL_LOOKUP condition to separate +mfsl flags from +t, allowing > (after refactoring) to avoid a needless call-return. > > Add a PREFIX_CACHED bit to remember that a pr-debug callsite is: > > - enabled, with +p > - wants a dynamic-prefix, with _INCL_LOOKUP > - was previously called > - was thus saved in the prefix cache. NOT YET. > > This allows (later) to cache part/all of the dynamic-prefix for each > pr_debug that gets called. > > NOTES: > > dyndbg's dynamic prefixing can get expensive; each enabled callsite's > prefix is sprintf'd into stack-mem, every time a pr_debug is called. > > A cache would help, if callsites mark _DPRINTK_FLAGS_PREFIX_CACHED > after saving the prefix string. But not yet. > > -t thread-id. not part of the "callsite" info, derived from current. > doesn't belong in the cache. it would be wrong. > can be done in outer: dynamic_emit_prefix() > > -mfsl module, function, source-file, line > we cache these, composed into a sub-string. > they are "lookups", currently to descriptor fields,. > could be accessor macros to "compressed" tables. > > All enabled together, they compose a prefix string like: > > # outer -----inner------------------- > "[tid] module:function:sourcfile:line: " s/sourcfile/sourcesfile/ > > So this patch extracts _DPRINTK_FLAGS_INCL_LOOKUP macro out of > _DPRINTK_FLAGS_INCL_ANY macro, then redefs latter. > > Next re-refactor dynamic_emit_prefix inner/outer fns accordingly. This commit introduces two things: - introduction of _DPRINTK_FLAGS_INCL_LOOKUP, used in a future patch - introduction of _DPRINTK_FLAGS_PREFIX_CACHED, not used in this series I don't think those changes are needed to fix DYNDBG_CLASSMAP, it seems to be an (unfinished?) optimization, so it could make sense to move it to an independent series. Please tell me if I overlooked something! > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > --- > include/linux/dynamic_debug.h | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > index c388ab05a6e1..82eabaa6e827 100644 > --- a/include/linux/dynamic_debug.h > +++ b/include/linux/dynamic_debug.h > @@ -38,11 +38,13 @@ struct _ddebug { > #define _DPRINTK_FLAGS_INCL_LINENO (1<<3) > #define _DPRINTK_FLAGS_INCL_TID (1<<4) > #define _DPRINTK_FLAGS_INCL_SOURCENAME (1<<5) > +#define _DPRINTK_FLAGS_PREFIX_CACHED (1<<7) Is there a reason to skip 1 << 6? I don't see any usage of this flag in this series, maybe you can skip it for now and introduce it with the actual implementation of the cache system? Also, I think it make sense to add some documentation on this define. All the other are controlled by the user, but PREFIX_CACHED is controlled by the "dyndbg core", so maybe add something like: /** * _DPRINTK_FLAGS_PREFIX_CACHED - Mark a printk prefix as cached * This bit is set by the callsite to avoid regenerating fixed * part of the prefix at each call */ > > -#define _DPRINTK_FLAGS_INCL_ANY \ > - (_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\ > - _DPRINTK_FLAGS_INCL_LINENO | _DPRINTK_FLAGS_INCL_TID |\ > - _DPRINTK_FLAGS_INCL_SOURCENAME) > +#define _DPRINTK_FLAGS_INCL_LOOKUP \ > + (_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME | \ > + _DPRINTK_FLAGS_INCL_SOURCENAME | _DPRINTK_FLAGS_INCL_LINENO) > +#define _DPRINTK_FLAGS_INCL_ANY \ > + (_DPRINTK_FLAGS_INCL_TID | _DPRINTK_FLAGS_INCL_LOOKUP) > > #if defined DEBUG > #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index c388ab05a6e1..82eabaa6e827 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -38,11 +38,13 @@ struct _ddebug { #define _DPRINTK_FLAGS_INCL_LINENO (1<<3) #define _DPRINTK_FLAGS_INCL_TID (1<<4) #define _DPRINTK_FLAGS_INCL_SOURCENAME (1<<5) +#define _DPRINTK_FLAGS_PREFIX_CACHED (1<<7) -#define _DPRINTK_FLAGS_INCL_ANY \ - (_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\ - _DPRINTK_FLAGS_INCL_LINENO | _DPRINTK_FLAGS_INCL_TID |\ - _DPRINTK_FLAGS_INCL_SOURCENAME) +#define _DPRINTK_FLAGS_INCL_LOOKUP \ + (_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME | \ + _DPRINTK_FLAGS_INCL_SOURCENAME | _DPRINTK_FLAGS_INCL_LINENO) +#define _DPRINTK_FLAGS_INCL_ANY \ + (_DPRINTK_FLAGS_INCL_TID | _DPRINTK_FLAGS_INCL_LOOKUP) #if defined DEBUG #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
Add _INCL_LOOKUP condition to separate +mfsl flags from +t, allowing (after refactoring) to avoid a needless call-return. Add a PREFIX_CACHED bit to remember that a pr-debug callsite is: - enabled, with +p - wants a dynamic-prefix, with _INCL_LOOKUP - was previously called - was thus saved in the prefix cache. NOT YET. This allows (later) to cache part/all of the dynamic-prefix for each pr_debug that gets called. NOTES: dyndbg's dynamic prefixing can get expensive; each enabled callsite's prefix is sprintf'd into stack-mem, every time a pr_debug is called. A cache would help, if callsites mark _DPRINTK_FLAGS_PREFIX_CACHED after saving the prefix string. But not yet. -t thread-id. not part of the "callsite" info, derived from current. doesn't belong in the cache. it would be wrong. can be done in outer: dynamic_emit_prefix() -mfsl module, function, source-file, line we cache these, composed into a sub-string. they are "lookups", currently to descriptor fields,. could be accessor macros to "compressed" tables. All enabled together, they compose a prefix string like: # outer -----inner------------------- "[tid] module:function:sourcfile:line: " So this patch extracts _DPRINTK_FLAGS_INCL_LOOKUP macro out of _DPRINTK_FLAGS_INCL_ANY macro, then redefs latter. Next re-refactor dynamic_emit_prefix inner/outer fns accordingly. Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- include/linux/dynamic_debug.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)