[PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module

Jim Cromie posted 31 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
Posted by Jim Cromie 2 months, 3 weeks ago
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@gmail.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
+ */
+#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);
 
-- 
2.51.1
Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
Posted by Jason Baron 2 months ago
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@gmail.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);
>
Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
Posted by jim.cromie@gmail.com 2 months ago
On Wed, Dec 10, 2025 at 11:43 AM Jason Baron <jbaron@akamai.com> wrote:
>
> Hi Jim,
>
> Very minor nit below about the kernel-doc ordering for args...
>

> > +/*
> > + * 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
>

I chose that doc ordering for clarity,  the easy ones 1st,
and @dst last since it gets the subrange info.
I think reordering might mean more words trying to connect
the pieces, and with less clarity.
It does work against the macro arg ordering,
which places @dst near the front,
I did that to follow  LHS = RHS(...)   convention.

Im happy to swap it around if anyone thinks that convention
should supercede these reasons,
but Im in NZ on vacation right now,
and I forgot to pull the latest rev off my desktop before I left.
so I dont want to fiddle with the slightly older copy I have locally,
and then have to isolate and fix whatever is different.

the same applies to the Documentation tweaks that Bagas noted.





> > +#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);
> >
>
Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
Posted by Jason Baron 2 months ago

On 12/10/25 1:33 AM, jim.cromie@gmail.com wrote:
> !-------------------------------------------------------------------|
>    This Message Is From an External Sender
>    This message came from outside your organization.
> |-------------------------------------------------------------------!
> 
> On Wed, Dec 10, 2025 at 11:43 AM Jason Baron <jbaron@akamai.com> wrote:
>>
>> Hi Jim,
>>
>> Very minor nit below about the kernel-doc ordering for args...
>>
> 
>>> +/*
>>> + * 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
>>
> 
> I chose that doc ordering for clarity,  the easy ones 1st,
> and @dst last since it gets the subrange info.
> I think reordering might mean more words trying to connect
> the pieces, and with less clarity.
> It does work against the macro arg ordering,
> which places @dst near the front,
> I did that to follow  LHS = RHS(...)   convention.
> 
> Im happy to swap it around if anyone thinks that convention
> should supercede these reasons,
> but Im in NZ on vacation right now,
> and I forgot to pull the latest rev off my desktop before I left.
> so I dont want to fiddle with the slightly older copy I have locally,
> and then have to isolate and fix whatever is different.
> 
> the same applies to the Documentation tweaks that Bagas noted.

Couldn't you then re-order the function args to match the doc order instead?

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);
>>>
>>

Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
Posted by jim.cromie@gmail.com 1 month, 4 weeks ago
On Thu, Dec 11, 2025 at 8:14 AM Jason Baron <jbaron@akamai.com> wrote:
>
>
>
> On 12/10/25 1:33 AM, jim.cromie@gmail.com wrote:
> > !-------------------------------------------------------------------|
> >    This Message Is From an External Sender
> >    This message came from outside your organization.
> > |-------------------------------------------------------------------!
> >
> > On Wed, Dec 10, 2025 at 11:43 AM Jason Baron <jbaron@akamai.com> wrote:
> >>
> >> Hi Jim,
> >>
> >> Very minor nit below about the kernel-doc ordering for args...
> >>
> >
> >>> +/*
> >>> + * 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
> >>
> >
> > I chose that doc ordering for clarity,  the easy ones 1st,
> > and @dst last since it gets the subrange info.
> > I think reordering might mean more words trying to connect
> > the pieces, and with less clarity.
> > It does work against the macro arg ordering,
> > which places @dst near the front,
> > I did that to follow  LHS = RHS(...)   convention.
> >
> > Im happy to swap it around if anyone thinks that convention
> > should supercede these reasons,
> > but Im in NZ on vacation right now,
> > and I forgot to pull the latest rev off my desktop before I left.
> > so I dont want to fiddle with the slightly older copy I have locally,
> > and then have to isolate and fix whatever is different.
> >
> > the same applies to the Documentation tweaks that Bagas noted.
>
> Couldn't you then re-order the function args to match the doc order instead?
>

As you might surmise, the code was written before the kdoc.
Since it is setting the @_dst, it feels like an assignment.
Therefore the LHS = RHS convention seemed pertinent,
and the macro args are ordered to conform to this.
For the (pseudo- since its not /** ) kdoc,
the linear explanation was simplest and clearest, ending with @_dst.

So I see these options (in my preferred order), please pick one.
1. leave as is
2. add an NB: that arg order differs from doc-order
3. change macro arg order
4. change kdoc arg order

If 2-4 can wait, I can do that trivially once Im home (in Jan)
Doing it now, from here, will require fiddling with git am on the mbox.gz
with which Ive had mixed results/troubles in the past.

thanks,
Jim

> 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);
> >>>
> >>
>
Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
Posted by Jason Baron 1 month, 4 weeks ago

On 12/10/25 3:21 PM, jim.cromie@gmail.com wrote:
> !-------------------------------------------------------------------|
>    This Message Is From an External Sender
>    This message came from outside your organization.
> |-------------------------------------------------------------------!
> 
> On Thu, Dec 11, 2025 at 8:14 AM Jason Baron <jbaron@akamai.com> wrote:
>>
>>
>>
>> On 12/10/25 1:33 AM, jim.cromie@gmail.com wrote:
>>> !-------------------------------------------------------------------|
>>>     This Message Is From an External Sender
>>>     This message came from outside your organization.
>>> |-------------------------------------------------------------------!
>>>
>>> On Wed, Dec 10, 2025 at 11:43 AM Jason Baron <jbaron@akamai.com> wrote:
>>>>
>>>> Hi Jim,
>>>>
>>>> Very minor nit below about the kernel-doc ordering for args...
>>>>
>>>
>>>>> +/*
>>>>> + * 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
>>>>
>>>
>>> I chose that doc ordering for clarity,  the easy ones 1st,
>>> and @dst last since it gets the subrange info.
>>> I think reordering might mean more words trying to connect
>>> the pieces, and with less clarity.
>>> It does work against the macro arg ordering,
>>> which places @dst near the front,
>>> I did that to follow  LHS = RHS(...)   convention.
>>>
>>> Im happy to swap it around if anyone thinks that convention
>>> should supercede these reasons,
>>> but Im in NZ on vacation right now,
>>> and I forgot to pull the latest rev off my desktop before I left.
>>> so I dont want to fiddle with the slightly older copy I have locally,
>>> and then have to isolate and fix whatever is different.
>>>
>>> the same applies to the Documentation tweaks that Bagas noted.
>>
>> Couldn't you then re-order the function args to match the doc order instead?
>>
> 
> As you might surmise, the code was written before the kdoc.
> Since it is setting the @_dst, it feels like an assignment.
> Therefore the LHS = RHS convention seemed pertinent,
> and the macro args are ordered to conform to this.
> For the (pseudo- since its not /** ) kdoc,
> the linear explanation was simplest and clearest, ending with @_dst.
> 
> So I see these options (in my preferred order), please pick one.
> 1. leave as is
> 2. add an NB: that arg order differs from doc-order
> 3. change macro arg order
> 4. change kdoc arg order
> 
> If 2-4 can wait, I can do that trivially once Im home (in Jan)
> Doing it now, from here, will require fiddling with git am on the mbox.gz
> with which Ive had mixed results/troubles in the past.
> 

Hi Jim,

I am fine leaving this as is, but I do feel like we should perhaps do at 
least #2 at some point, to clarify things.


Thanks,

-Jason








> thanks,
> Jim
> 
>> 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);
>>>>>
>>>>
>>

Re: [PATCH v6 16/31] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
Posted by jim.cromie@gmail.com 1 month, 3 weeks ago
On Sat, Dec 13, 2025 at 5:06 AM Jason Baron <jbaron@akamai.com> wrote:
>
>
>
> On 12/10/25 3:21 PM, jim.cromie@gmail.com wrote:
> > !-------------------------------------------------------------------|
> >    This Message Is From an External Sender
> >    This message came from outside your organization.
> > |-------------------------------------------------------------------!
> >
> > On Thu, Dec 11, 2025 at 8:14 AM Jason Baron <jbaron@akamai.com> wrote:
> >>
> >>
> >>
> >> On 12/10/25 1:33 AM, jim.cromie@gmail.com wrote:
> >>> !-------------------------------------------------------------------|
> >>>     This Message Is From an External Sender
> >>>     This message came from outside your organization.
> >>> |-------------------------------------------------------------------!
> >>>
> >>> On Wed, Dec 10, 2025 at 11:43 AM Jason Baron <jbaron@akamai.com> wrote:
> >>>>
> >>>> Hi Jim,
> >>>>
> >>>> Very minor nit below about the kernel-doc ordering for args...
> >>>>
> >>>
> >>>>> +/*
> >>>>> + * 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
> >>>>
> >>>
> >>> I chose that doc ordering for clarity,  the easy ones 1st,
> >>> and @dst last since it gets the subrange info.
> >>> I think reordering might mean more words trying to connect
> >>> the pieces, and with less clarity.
> >>> It does work against the macro arg ordering,
> >>> which places @dst near the front,
> >>> I did that to follow  LHS = RHS(...)   convention.
> >>>
> >>> Im happy to swap it around if anyone thinks that convention
> >>> should supercede these reasons,
> >>> but Im in NZ on vacation right now,
> >>> and I forgot to pull the latest rev off my desktop before I left.
> >>> so I dont want to fiddle with the slightly older copy I have locally,
> >>> and then have to isolate and fix whatever is different.
> >>>
> >>> the same applies to the Documentation tweaks that Bagas noted.
> >>
> >> Couldn't you then re-order the function args to match the doc order instead?
> >>
> >
> > As you might surmise, the code was written before the kdoc.
> > Since it is setting the @_dst, it feels like an assignment.
> > Therefore the LHS = RHS convention seemed pertinent,
> > and the macro args are ordered to conform to this.
> > For the (pseudo- since its not /** ) kdoc,
> > the linear explanation was simplest and clearest, ending with @_dst.
> >
> > So I see these options (in my preferred order), please pick one.
> > 1. leave as is
> > 2. add an NB: that arg order differs from doc-order
> > 3. change macro arg order
> > 4. change kdoc arg order
> >
> > If 2-4 can wait, I can do that trivially once Im home (in Jan)
> > Doing it now, from here, will require fiddling with git am on the mbox.gz
> > with which Ive had mixed results/troubles in the past.
> >
>
> Hi Jim,
>
> I am fine leaving this as is, but I do feel like we should perhaps do at
> least #2 at some point, to clarify things.
>
>

Im redoing the set anyway, so I'll do either 2 or 3.

thx


> Thanks,
>
> -Jason
>
>
>
>
>
>
>
>
> > thanks,
> > Jim
> >
> >> 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);
> >>>>>
> >>>>
> >>
>