| Message ID | 20250320185238.447458-15-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 : > Split dynamic_emit_prefix() to separate out _INCL_LOOKUPs: > > 1. keep dynamic_emit_prefix() static inline > check _INCL_ANY flags before calling 2 > > 2. __dynamic_emit_prefix() > prints [TID] or <intr> and trailing space if +t flag > check _INCL_LOOKUP flags before calling 3 > > 3. __dynamic_emit_lookup() > prints ONLY module, function, src, line, and trailing space > TID isn't "callsite" specific info. > result is "cacheable" > > Notes: > > 2,3 are gated, only called when theyve something to emit, so they just > add trailing space. This obsoletes the pos_after_tid var and logic. > > __dynamic_emit_lookup() adds line too, so the result is "whole". > While this would enlarge a naive cache vs add-line-after-caching, we > dont even have a naive one yet. > > And some clever indexing on store() might be able to fold the flags > setting in, such that the prefix stored with +mf flags only (-l), > could be returned for all pr_debugs in that function which also had > +mf flags. While still supporting +mfsl prefixes (with cache > expansion) as they're used. Like the previous patch: I think this should be a separate series. > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > --- > lib/dynamic_debug.c | 40 +++++++++++++++++++++++++--------------- > 1 file changed, 25 insertions(+), 15 deletions(-) > > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index 663c125006d0..f7ec2365ab40 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c > @@ -850,19 +850,8 @@ static int remaining(int wrote) > return 0; > } > > -static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf) > +static int __dynamic_emit_lookup(const struct _ddebug *desc, char *buf, int pos) > { > - int pos_after_tid; > - int pos = 0; > - > - if (desc->flags & _DPRINTK_FLAGS_INCL_TID) { > - if (in_interrupt()) > - pos += snprintf(buf + pos, remaining(pos), "<intr> "); > - else > - pos += snprintf(buf + pos, remaining(pos), "[%d] ", > - task_pid_vnr(current)); > - } > - pos_after_tid = pos; > if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME) > pos += snprintf(buf + pos, remaining(pos), "%s:", > desc->modname); > @@ -875,8 +864,29 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf) > if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO) > pos += snprintf(buf + pos, remaining(pos), "%d:", > desc->lineno); > - if (pos - pos_after_tid) > - pos += snprintf(buf + pos, remaining(pos), " "); > + > + /* cuz LOOKUP, we've emitted, so add trailing space if room */ s/cuz/because/ > + if (remaining(pos)) > + buf[pos++] = ' '; > + > + return pos; > +} > + > +static char *__dynamic_emit_prefix(struct _ddebug *desc, char *buf) > +{ > + int pos = 0; > + > + if (desc->flags & _DPRINTK_FLAGS_INCL_TID) { > + if (in_interrupt()) > + pos += snprintf(buf + pos, remaining(pos), "<intr> "); > + else > + pos += snprintf(buf + pos, remaining(pos), "[%d] ", > + task_pid_vnr(current)); > + } > + > + if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_LOOKUP)) > + pos += __dynamic_emit_lookup(desc, buf, pos); > + > if (pos >= PREFIX_SIZE) > buf[PREFIX_SIZE - 1] = '\0'; > > @@ -885,7 +895,7 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf) > > static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf) > { > - if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_ANY)) > + if (desc->flags & _DPRINTK_FLAGS_INCL_ANY) Why do you remove the unlikely here? > return __dynamic_emit_prefix(desc, buf); > return buf; > }
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 663c125006d0..f7ec2365ab40 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -850,19 +850,8 @@ static int remaining(int wrote) return 0; } -static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf) +static int __dynamic_emit_lookup(const struct _ddebug *desc, char *buf, int pos) { - int pos_after_tid; - int pos = 0; - - if (desc->flags & _DPRINTK_FLAGS_INCL_TID) { - if (in_interrupt()) - pos += snprintf(buf + pos, remaining(pos), "<intr> "); - else - pos += snprintf(buf + pos, remaining(pos), "[%d] ", - task_pid_vnr(current)); - } - pos_after_tid = pos; if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME) pos += snprintf(buf + pos, remaining(pos), "%s:", desc->modname); @@ -875,8 +864,29 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf) if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO) pos += snprintf(buf + pos, remaining(pos), "%d:", desc->lineno); - if (pos - pos_after_tid) - pos += snprintf(buf + pos, remaining(pos), " "); + + /* cuz LOOKUP, we've emitted, so add trailing space if room */ + if (remaining(pos)) + buf[pos++] = ' '; + + return pos; +} + +static char *__dynamic_emit_prefix(struct _ddebug *desc, char *buf) +{ + int pos = 0; + + if (desc->flags & _DPRINTK_FLAGS_INCL_TID) { + if (in_interrupt()) + pos += snprintf(buf + pos, remaining(pos), "<intr> "); + else + pos += snprintf(buf + pos, remaining(pos), "[%d] ", + task_pid_vnr(current)); + } + + if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_LOOKUP)) + pos += __dynamic_emit_lookup(desc, buf, pos); + if (pos >= PREFIX_SIZE) buf[PREFIX_SIZE - 1] = '\0'; @@ -885,7 +895,7 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf) static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf) { - if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_ANY)) + if (desc->flags & _DPRINTK_FLAGS_INCL_ANY) return __dynamic_emit_prefix(desc, buf); return buf; }
Split dynamic_emit_prefix() to separate out _INCL_LOOKUPs: 1. keep dynamic_emit_prefix() static inline check _INCL_ANY flags before calling 2 2. __dynamic_emit_prefix() prints [TID] or <intr> and trailing space if +t flag check _INCL_LOOKUP flags before calling 3 3. __dynamic_emit_lookup() prints ONLY module, function, src, line, and trailing space TID isn't "callsite" specific info. result is "cacheable" Notes: 2,3 are gated, only called when theyve something to emit, so they just add trailing space. This obsoletes the pos_after_tid var and logic. __dynamic_emit_lookup() adds line too, so the result is "whole". While this would enlarge a naive cache vs add-line-after-caching, we dont even have a naive one yet. And some clever indexing on store() might be able to fold the flags setting in, such that the prefix stored with +mf flags only (-l), could be returned for all pr_debugs in that function which also had +mf flags. While still supporting +mfsl prefixes (with cache expansion) as they're used. Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- lib/dynamic_debug.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-)