All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jim Cromie" <jim.cromie@gmail.com>
Cc: linux-modules@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v6 15/24] dyndbg: refactor param_set_dyndbg_classes and below
Date: Wed, 08 Jul 2026 02:29:13 +0000	[thread overview]
Message-ID: <20260708022913.E3D9A1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260707-dd-maint-2-v6-15-381f3edb0045@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] param_set_dyndbg_module_classes() fails to initialize debug queries for a newly probed module because it incorrectly uses the global class parameter state as the baseline (`old_bits`).
--

--- Patch [15]: [PATCH v6 15/24] dyndbg: refactor param_set_dyndbg_classes and below ---
commit 041ecc33e02248dbfd09801bededdf6047b80f6e
Author: Jim Cromie <jim.cromie@gmail.com>

dyndbg: refactor param_set_dyndbg_classes and below

This commit refactors the param_set_dyndbg_classes callchain to allow
module-specific settings by passing a modname parameter down to
ddebug_apply_class_bitmap and ddebug_exec_queries. It also adds a check
to announce bit changes only when they actually change.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index ce42e03f1600a..0fc9cd14e2d20 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
[ ... ]
> @@ -682,8 +692,8 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
>  				KP_NAME(kp), inrep, CLASSMAP_BITMASK(map->length));
>  			inrep &= CLASSMAP_BITMASK(map->length);
>  		}
> -		v2pr_info("bits:%lx > %s\n", inrep, KP_NAME(kp));
> -		totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits);
> +		v2pr_info("bits:0x%lx > %s.%s\n", inrep, mod_name ?: "*", KP_NAME(kp));
> +		totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, mod_name);
>  		*dcp->bits = inrep;
>  		break;
>  	case DD_CLASS_TYPE_LEVEL_NUM:
> @@ -696,7 +706,7 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
>  		old_bits = CLASSMAP_BITMASK(*dcp->lvl);
>  		new_bits = CLASSMAP_BITMASK(inrep);
>  		v2pr_info("lvl:%ld bits:0x%lx > %s\n", inrep, new_bits, KP_NAME(kp));
> -		totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits);
> +		totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits, mod_name);

[Severity: Medium]
Does this skip enabling debug queries for newly probed modules if the global
state already has those bits set?

When param_set_dyndbg_module_classes() is called for a newly probed module
(mod_name != NULL), it passes the global class parameter state as the old
state (dcp->bits and old_bits respectively) instead of 0.

If a global parameter was previously set (e.g., drm.debug=2), those bits are
already present in the global state. The optimization loop in
ddebug_apply_class_bitmap() checks this:

lib/dynamic_debug.c:ddebug_apply_class_bitmap() {
    ...
    for (bi = 0; bi < map->length; bi++) {
        if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
            continue;
    ...
}

Because the new module's target state matches the global state, it skips
executing the queries, even though the newly loaded module actually starts
with a default state of 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-dd-maint-2-v6-0-381f3edb0045@gmail.com?part=15

  reply	other threads:[~2026-07-08  2:29 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08  2:18 [PATCH v6 00/24] fix dynamic-debug classmaps API for DRM Jim Cromie
2026-07-08  2:18 ` [PATCH v6 01/24] vmlinux.lds.h: refactor BOUNDED_SECTION_* macros into bounded_sections.lds.h Jim Cromie
2026-07-08  2:18 ` [PATCH v6 02/24] vmlinux.lds.h: drop unused HEADERED_SECTION* macros Jim Cromie
2026-07-08  2:18 ` [PATCH v6 03/24] vmlinux.lds.h: Fix ALIGN(8) omission causing NULL ptr on i386 Jim Cromie
2026-07-08  2:18 ` [PATCH v6 04/24] vmlinux.lds.h: remove redundant ALIGN(8) directives Jim Cromie
2026-07-08  2:18 ` [PATCH v6 05/24] dyndbg.lds.S: fix lost dyndbg sections in modules Jim Cromie
2026-07-08  2:18 ` [PATCH v6 06/24] dyndbg: factor ddebug_match_desc out from ddebug_change Jim Cromie
2026-07-08  2:18 ` [PATCH v6 07/24] dyndbg: add stub macro for DECLARE_DYNDBG_CLASSMAP Jim Cromie
2026-07-08  2:18 ` [PATCH v6 08/24] dyndbg: reword "class unknown," to "class:_UNKNOWN_" Jim Cromie
2026-07-08  2:18 ` [PATCH v6 09/24] dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code Jim Cromie
2026-07-08  2:18 ` [PATCH v6 10/24] dyndbg: drop NUM_TYPE_ARGS Jim Cromie
2026-07-08  2:24   ` sashiko-bot
2026-07-08 19:30     ` jim.cromie
2026-07-08  2:18 ` [PATCH v6 11/24] dyndbg: bump num-tokens in a query-cmd from 9 to 15 Jim Cromie
2026-07-08  2:18 ` [PATCH v6 12/24] dyndbg: reduce verbose/debug clutter Jim Cromie
2026-07-08  2:18 ` [PATCH v6 13/24] lib/parser: add match_wildcard_hyphen() for agnostic matching Jim Cromie
2026-07-08  2:18 ` [PATCH v6 14/24] dyndbg: use KBUILD_MODFILE for unique builtin module names Jim Cromie
2026-07-08  2:18 ` [PATCH v6 15/24] dyndbg: refactor param_set_dyndbg_classes and below Jim Cromie
2026-07-08  2:29   ` sashiko-bot [this message]
2026-07-08 19:28     ` jim.cromie
2026-07-08  2:18 ` [PATCH v6 16/24] dyndbg: tighten fn-sig of ddebug_apply_class_bitmap Jim Cromie
2026-07-08  2:18 ` [PATCH v6 17/24] dyndbg: replace classmap list with an array-slice Jim Cromie
2026-07-08  2:18 ` [PATCH v6 18/24] dyndbg: macrofy a 2-index for-loop pattern Jim Cromie
2026-07-08  2:18 ` [PATCH v6 19/24] dyndbg: pin class param storage to u32 Jim Cromie
2026-07-08  2:37   ` sashiko-bot
2026-07-08 20:48     ` jim.cromie
2026-07-08  2:18 ` [PATCH v6 20/24] dyndbg,module: make proper substructs in _ddebug_info Jim Cromie
2026-07-08  2:30   ` sashiko-bot
2026-07-08  2:18 ` [PATCH v6 21/24] dyndbg: move mod_name down from struct ddebug_table to _ddebug_info Jim Cromie
2026-07-08  2:18 ` [PATCH v6 22/24] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module Jim Cromie
2026-07-08  2:18 ` [PATCH v6 23/24] dyndbg: change __dynamic_func_call_cls* macros into expressions Jim Cromie
2026-07-08  2:41   ` sashiko-bot
2026-07-08  2:18 ` [PATCH v6 24/24] dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP Jim Cromie
2026-07-08  2:36   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260708022913.E3D9A1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jim.cromie@gmail.com \
    --cc=linux-modules@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.