linux-kernel - Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fcb2532d-5627-4bc3-a990-ed361b56ccd3@akamai.com>
Date: Tue, 9 Dec 2025 17:43:15 -0500
From: Jason Baron <jbaron@...mai.com>
To: Jim Cromie <jim.cromie@...il.com>, <linux-kernel@...r.kernel.org>,
        <dri-devel@...ts.freedesktop.org>, <gregkh@...uxfoundation.org>
CC: <ukaszb@...omium.org>, <louis.chauvet@...tlin.com>
Subject: Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to
 ddebug_add_module

Hi Jim,

Very minor nit below about the kernel-doc ordering for args...

On 11/18/25 3:18 PM, Jim Cromie wrote:
> !-------------------------------------------------------------------|
>    This Message Is From an External Sender
>    This message came from outside your organization.
> |-------------------------------------------------------------------!
> 
> 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).  We will
> reuse this macro shortly.
> 
> 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 already segments the builtin pr_debug
> descriptors on their mod_name boundaries.
> 
> Signed-off-by: Jim Cromie <jim.cromie@...il.com>
> ---
> Ive review checkpatch complaints:
> all are lvalues, and not issues.
> CHECK: Macro argument reuse '_dst' - possible side-effects?
> CHECK: Macro argument reuse '_sp' - possible side-effects?
> CHECK: Macro argument reuse '_vec' - possible side-effects?
> ---
>   lib/dynamic_debug.c | 57 ++++++++++++++++++++++++++-------------------
>   1 file changed, 33 insertions(+), 24 deletions(-)
> 
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index c9377a444fc8..49de591f036a 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -170,8 +170,8 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
>   }
>   
>   static struct _ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
> -							const char *class_string,
> -							int *class_id)
> +							 const char *class_string,
> +							 int *class_id)
>   {
>   	struct _ddebug_class_map *map;
>   	int i, idx;
> @@ -1247,30 +1247,35 @@ 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);
>   }
>   
> +/*
> + * Walk the @_box->@_vec member, over @_vec.start[0..len], and find
> + * the contiguous subrange of elements matching on ->mod_name.  Copy
> + * the subrange into @_dst.  This depends on vars defd by caller.
> + *
> + * @_i:   caller provided counter var, init'd by macro
> + * @_sp:  cursor into @_vec.
> + * @_box: contains member named @_vec
> + * @_vec: member-name of a type with: .start .len fields.
> + * @_dst: an array-ref: to remember the module's subrange
> + */

Not sure if the odering matters for the docs, but it makes it a bit 
harder read when these don't go in order.

Thanks,

-Jason


> +#define dd_mark_vector_subrange(_i, _dst, _sp, _box, _vec) ({		\
> +	typeof(_dst) __dst = (_dst);					\
> +	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.
> @@ -1278,6 +1283,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;
> @@ -1300,6 +1307,8 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
>   
>   	INIT_LIST_HEAD(&dt->link);
>   
> +	dd_mark_vector_subrange(i, dt, cm, di, maps);
> +
>   	if (di->maps.len)
>   		ddebug_attach_module_classes(dt, di);
>