From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=100.103.45.18 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1793219116; b=nzagL5pt66vvJR+rZySq/fanR9TSEgRpz7JBY5hDEh5eW4PZyyvVBrr0MYaK2bIT1a6+ cV9rKjVvYOZ1eRjtmeRiR38lY0vI2JZppXNPqWmsf+Bxw1HS5KMel67acMMNtxNY+HIIM uiSwHbzMz1tke4rPe744abVL7Z3Al0JAW5NeniSa/4Ms+Y4eAooiK0nqfdIUEC5jIf+w7 oSA+F2JmlIPIxg8CHs1aiag51b5GGT7wnue1mGjF7or9zObqHm9WBbZfHKeH3VFlUVKPJ RYqtTBnpen/1qXFJ+91bZoS9I/qSHRXfvxyhdFnrS6FIU3qJ0b6Plgzofk+le54fBcQ== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1793219116; h=Received:DKIM-Signature:Message-ID:From:Subject:Reply-To:To:Cc: In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date; bh=2k29I4YIPJKvXD8D5WaACCIQj3alsOfSXoDVRD69hJA=; b=IADvLU2MjXVMSGz6u3oFKmBy0avewgyunWb2wOoLXXNsvZbBoDiRiDT2xVTxUS0vNPJF xAQcCw09iLsdwdeA2YjgWhFPftkjBfzVQhZ+d2OJEXIOeM4OTFD1lezAdABXO85ONctZ8 Boe2dsHfjP6suR3f4Il+s2ur4DRSlB5hfhZKXi7y1Hdk6tW0LzjJdRCWYW3MBzj+JVcxc +pMvmXSSN0149lceMVQ/64razwqADC7rVR0s85oaswqB60TePoHDHGFdrABDgSkoA4Zu5 ecDGAIPJNzpWhuW/4VpmPYe1i3VGvXqO/FQk0RhBjujJt1UcMWPtuF/sRktl+jr9o1w== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=HsjjoPQX; arc=none smtp.remote-ip=100.103.45.18 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 10520606A31A; Wed, 28 Oct 2026 20:25:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key, unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=HsjjoPQX Received: by smtp.kernel.org (Postfix) with ESMTPSA id 089691F01825; Wed, 28 Oct 2026 20:25:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1793219115; bh=2k29I4YIPJKvXD8D5WaACCIQj3alsOfSXoDVRD69hJA=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=HsjjoPQXvvp10U7GSXIqo2E5g0/J62pvY/a2+Hy2HGBIL2vgggF1gO6UqvgqekJxL by113CvFNRK8PF9u3IzrkCeMgOrjj3y5h2NCth0s9+RRDYsB2qjoCo0nhBvLrJzMWU ir9TQwiuprA400e7yqFCEhNqxdq5qK4gdivVm8k9u8QI2Zu5IcbdA4JC5aeq6zHeY/ diSM8esKSkSF+l+1ARsBpxXnk10r6ljFvLzdCRn3g0xwfBRvLugbbnWaLvw6yoJYoC gUfWf9GvIxJQQJGXnQagghpZ8WP/5TSdqsaEelC5thd5la0FlkIwhmlk0OhrRPnnkK iyiwynxEOHFog== Message-ID: From: sashiko-bot@kernel.org Subject: Re: [PATCH v13 22/39] dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP Reply-To: sashiko-reviews@lists.linux.dev To: "Jim Cromie" , "Jason Baron" , "Shuah Khan" , "Andrew Morton" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "David Airlie" , "Simona Vetter" , "Arnd Bergmann" , "Luis Chamberlain" , "Petr Pavlu" , "Daniel Gomez" , "Sami Tolvanen" , "Aaron Tomlin" , "Jonathan Corbet" , "Greg Kroah-Hartman" , "Nathan Chancellor" , "Nicolas Schier" , "Shuah Khan" , "Randy Dunlap" , "Rafael J. Wysocki" , "Pavel Machek" , "Len Brown" , "Jonathan Corbet" , "Petr Mladek" , "Steven Rostedt" , "John Ogness" , "Sergey Senozhatsky" , =?utf-8?b?VGhvbWFzIFdlacOfc2NodWg=?= , "Jason A. Donenfeld" Cc: dri-devel@lists.freedesktop.org, linux-modules@vger.kernel.org, linux-kbuild@vger.kernel.org, "Jason Baron" , "Louis Chauvet" In-Reply-To: <20261027-dd-cmap-part2-clean-v13-22-9267923be612@gmail.com> References: <20261027-dd-cmap-part2-clean-v13-0-9267923be612@gmail.com> <20261027-dd-cmap-part2-clean-v13-22-9267923be612@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Wed, 28 Oct 2026 20:25:13 +0000 X-Mailing-List: linux-modules@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: - [Low] dyndbg: unused ddebug_class_param_to_bits() and unclamped init in d= debug_sync_classbits() -- commit 061cb31380a7641ad87f8c44222d55e83ea7da16 Author: Jim Cromie dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP This commit replaces DECLARE_DYNDBG_CLASSMAP with DYNAMIC_DEBUG_CLASSMAP_DEFINE and DYNAMIC_DEBUG_CLASSMAP_USE to support exporting a classmap from a core module and referencing it from user module= s. At module load time, dynamic debug scans both classmap sections and syncs t= he initial class bitmap from the controlling kernel parameter. > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index 18289ca7170b7..ad86365f2474f 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c [ ... ] > @@ -1174,6 +1213,163 @@ static const struct proc_ops proc_fops =3D { > .proc_write =3D ddebug_proc_write > }; > =20 > +#define vpr_cm_info(cm_p, msg_fmt, ...) ({ \ > + struct ddebug_class_map const *_cm =3D cm_p; \ > + v2pr_info(msg_fmt "%s [%d..%d] %s..%s\n", ##__VA_ARGS__, \ > + _cm->mod_name, _cm->base, _cm->base + _cm->length, \ > + _cm->class_names[0], _cm->class_names[_cm->length - 1]); \ > + }) > + > +/* > + * Modules which define classmaps get them initialized by > + * param-callback via module.c:parse_one. Modules which use other's > + * classmaps must be initialized explicitly. > + */ > +static inline u32 ddebug_class_param_to_bits(const struct ddebug_class_p= aram *dcp) > +{ > + const struct ddebug_class_map *map =3D dcp->map; > + > + switch (map->map_type) { > + case DD_CLASS_TYPE_DISJOINT_BITS: > + return *dcp->bits & CLASSMAP_BITMASK(map->length); > + case DD_CLASS_TYPE_LEVEL_NUM: > + return CLASSMAP_BITMASK(min_t(u32, *dcp->lvl, map->length)); > + default: > + return 0; > + } > +} > + > +static void __maybe_unused ddebug_class_param_clamp_input(u32 *inrep, co= nst struct kernel_param *kp) > +{ > + const struct ddebug_class_param *dcp =3D kp->arg; > + const struct ddebug_class_map *map =3D dcp->map; > + > + switch (map->map_type) { > + case DD_CLASS_TYPE_DISJOINT_BITS: > + /* expect bits. mask and warn if too many */ > + if (*inrep & ~CLASSMAP_BITMASK(map->length)) { > + pr_warn("%s: input: 0x%x exceeds mask: 0x%x, masking\n", > + KP_NAME(kp), *inrep, CLASSMAP_BITMASK(map->length)); > + *inrep &=3D CLASSMAP_BITMASK(map->length); > + } > + break; > + case DD_CLASS_TYPE_LEVEL_NUM: > + /* input is bitpos, of highest verbosity to be enabled */ > + if (*inrep > map->length) { > + pr_warn("%s: level:%d exceeds max:%d, clamping\n", > + KP_NAME(kp), *inrep, map->length); > + *inrep =3D map->length; > + } > + break; > + } > +} > + > +/* > + * Called for class-users and native classmaps during module load. > + * > + * Initial sync assumes 0 as old_bits, so it only asserts (+p) classes > + * explicitly set by the parameter. -DDEBUGs invalidate this assumption > + * for some pr_debugs; a user can override the developer/builder choice > + * by toggling the param at runtime. > + * > + * Parameters are sampled via READ_ONCE without ddebug_lock; > + * holding ddebug_lock here would deadlock against ddebug_change(). > + */ > +static void ddebug_sync_classbits(const struct kernel_param *kp, const c= har *modname) > +{ > + const struct ddebug_class_param *dcp =3D kp->arg; > + u32 val, new_bits; > + > + if (!dcp || !dcp->map) > + return; > + > + switch (dcp->map->map_type) { > + case DD_CLASS_TYPE_DISJOINT_BITS: > + val =3D READ_ONCE(*dcp->bits); > + new_bits =3D val; > + v2pr_info(" %s: classbits: 0x%x\n", KP_NAME(kp), new_bits); > + ddebug_apply_class_bitmap(dcp, &new_bits, 0, modname); > + break; > + case DD_CLASS_TYPE_LEVEL_NUM: > + val =3D READ_ONCE(*dcp->lvl); > + new_bits =3D CLASSMAP_BITMASK(val); > + v2pr_info(" %s: lvl:%d bits:0x%x\n", KP_NAME(kp), val, new_bits); > + ddebug_apply_class_bitmap(dcp, &new_bits, 0, modname); > + break; [Severity: Low] Is ddebug_class_param_to_bits() intended to be used here, or should it be dropped? It is introduced in this patch, but never called here or in the re= st of the series (while ddebug_class_param_clamp_input() is only wired into param_set_dyndbg_module_classes() in the later commit "dyndbg-API: promote DYNAMIC_DEBUG_CLASSMAP_PARAM to API"). Also, in lib/test_dynamic_debug.c (and later in include/linux/dynamic_debug= .h when promoted in "dyndbg-API: promote DYNAMIC_DEBUG_CLASSMAP_PARAM to API"), DYNAMIC_DEBUG_CLASSMAP_PARAM initializes the backing u32 to ~0 when DEBUG is defined: #ifdef DEBUG #define DYNAMIC_DEBUG_CLASSMAP_PARAM(_model, _flags) \ DYNAMIC_DEBUG_CLASSMAP_PARAM_(_model, _flags, ~0) #else #define DYNAMIC_DEBUG_CLASSMAP_PARAM(_model, _flags) \ DYNAMIC_DEBUG_CLASSMAP_PARAM_(_model, _flags, 0) #endif Because ddebug_sync_classbits() reads *dcp->bits and *dcp->lvl directly without masking or clamping them to dcp->map->length, won't param_get_dyndbg_classes() report 0xffffffff for DD_CLASS_TYPE_DISJOINT_BITS (instead of CLASSMAP_BITMASK(map->length)) and 4294967295 for DD_CLASS_TYPE_LEVEL_NUM (instead of map->length) when built with -DDEBUG? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20261027-dd-cmap-pa= rt2-clean-v13-0-9267923be612@gmail.com?part=3D22