[PATCH v2 14/59] dyndbg: split _emit_lookup() out of dynamic_emit_prefix()

[Louis Chauvet]

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 at 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;
>   }

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com