Re: [PATCH v2 15/59] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module

[Louis Chauvet <louis.chauvet@bootlin.com>]

Le 20/03/2025 à 19:51, Jim Cromie a écrit :
> The body of ddebug_attach_module_classes() is dominated by a
> code-block that finds the contiguous subrange of classmaps matching on
> modname, and saves it into the ddebug_table's info record.
> 
> Implement this block in a macro to accommodate different component
> vectors in the "box" (as named in the for_subvec macro).
> 
> And hoist its invocation out of ddebug_attach_module_classes() up into
> ddebug_add_module().  This moves the filtering step up closer to
> dynamic_debug_init(), which effectively does the same for builtin
> pr_debug descriptors; segmenting them into subranges by modname.
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
  >
> ---
>   lib/dynamic_debug.c | 57 ++++++++++++++++++++++++++++-----------------
>   1 file changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index f7ec2365ab40..192783ff7b98 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -1242,30 +1242,34 @@ static const struct proc_ops proc_fops = {
>   
>   static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
>   {
> -	struct ddebug_class_map *cm;
> -	int i, nc = 0;
> -
> -	/*
> -	 * Find this module's classmaps in a subrange/wholerange of
> -	 * the builtin/modular classmap vector/section.  Save the start
> -	 * and length of the subrange at its edges.
> -	 */
> -	for_subvec(i, cm, di, maps) {
> -		if (!strcmp(cm->mod_name, dt->mod_name)) {
> -			if (!nc) {
> -				v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
> -					  i, cm->mod_name, cm->base, cm->length, cm->map_type);
> -				dt->info.maps.start = cm;
> -			}
> -			nc++;
> -		}
> -	}
> -	if (nc) {
> -		dt->info.maps.len = nc;
> -		vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
> -	}
> +	vpr_info("module:%s attached %d classes\n", dt->mod_name, dt->info.maps.len);
>   }
>   
> +/*
> + * scan the named array: @_vec, ref'd from inside @_box, for the
> + * start,len of the sub-array of elements matching on ->mod_name;
> + * remember them in _dst.  Macro depends upon the fields being in both
> + * _box and _dst.
> + * @_i:   caller provided counter var, init'd by macro
> + * @_sp:  cursor into @_vec.
> + * @_box: ptr to a struct with @_vec, num__##@_vec, mod_name fields.
> + * @_vec: name of ref into array[T] of builtin/modular __section data.
> + * @_dst: ptr to struct with @_vec and num__##@_vec fields, both updated.

I think you forgot to update this documentation with the new vector 
structure (num__##@_vec does not exists anymore)

> + */
> +#define dd_mark_vector_subrange(_i, _dst, _sp, _box, _vec) ({	\
> +	int nc = 0;							\
> +	for_subvec(_i, _sp, _box, _vec) {				\
> +		if (!strcmp((_sp)->mod_name, (_dst)->mod_name)) {	\
> +			if (!nc++)					\
> +				(_dst)->info._vec.start = (_sp);	\
> +		} else {						\
> +			if (nc)						\
> +				break; /* end of consecutive matches */ \
> +		}							\
> +	}								\
> +	(_dst)->info._vec.len = nc;					\
> +})
> +
>   /*
>    * Allocate a new ddebug_table for the given module
>    * and add it to the global list.
> @@ -1273,6 +1277,8 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
>   static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
>   {
>   	struct ddebug_table *dt;
> +	struct ddebug_class_map *cm;
> +	int i;
>   
>   	if (!di->descs.len)
>   		return 0;
> @@ -1294,6 +1300,13 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
>   	dt->info = *di;
>   
>   	INIT_LIST_HEAD(&dt->link);
> +	/*
> +	 * for builtin modules, ddebug_init() insures that the di
> +	 * cursor marks just the module's descriptors, but it doesn't
> +	 * do so for the builtin class _maps & _users.  find the

Nitpick, the _users builtin class is only introduced in patch 19/56, 
maybe update the documentation there.

> +	 * start,len of the vectors by mod_name, save to dt.
> +	 */
> +	dd_mark_vector_subrange(i, dt, cm, di, maps);
>   
>   	if (di->maps.len)
>   		ddebug_attach_module_classes(dt, di);

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