On Wed, Sep 23, 2026 at 4:45 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [Medium] selftests: classmap inheritance hash check excludes submod
> - [Low] selftests: undocumented uncommenting of grammar testcase
> --
>
> commit 7d522a72bd526c1e41ef1a1e604e466e1e729843
> Author: Jim Cromie <[email protected]>
>
> selftests/dyndbg: Enable FT_classmap_inheritance
>
> Exercise classmap state inheritance between supermod and submod,
> verifying that class settings and controlling parameter states propagate
> from parent classmap definitions to dependent class users upon module
> load and during runtime reconfiguration.
>
> > diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> > b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> > index c279c795867b6..5a805cb30e6cd 100755
> > --- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> > +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> [ ... ]
> > @@ -189,7 +190,7 @@ function FT_grammar_ok {
> >
> > # use 4 keywords (max 9 words inc flags)
> > ddcmd "module foo file bar.c func buz class D2_CORE +_" # 4 keywords
> > - #ddcmd "module foo file bar.c func buz class D2 line 100 +_" # 5
> > keywords
> > + ddcmd "module foo file bar.c func buz class D2 line 100 +_" # 5
> > keywords
>
> [Severity: Low]
> Was this undocumented uncommenting of a grammar testcase intentional?
>
> It modifies the behavior of an existing, separate test in FT_grammar_ok
> without
> documenting it in the commit message.
>
> [ ... ]
> > @@ -291,6 +292,78 @@ function FT_basic_queries {
> > ddcmd "file $f =_" "$f"
> > }
> >
> > +function FT_classmap_inheritance {
> [ ... ]
> > + # Real-time mathematical proof that load-time (modprobe) parameter
> > parsing
> > + # and runtime (sysfs write) parameter configurations are perfectly
> > equivalent!
> > + local hash_modprobe=$(slice_and_hash_ddctrl '\[test_dynamic_debug\]')
> > +
> > + # Fresh load with default parameters, then configure them dynamically
> > at runtime
> > + ifrmmod test_dynamic_debug_submod
> > + ifrmmod test_dynamic_debug
> > + my_modprobe test_dynamic_debug
> > + my_modprobe test_dynamic_debug_submod
> > + echo 0x16 > /sys/module/test_dynamic_debug/parameters/p_disjoint_bits
> > + echo 5 > /sys/module/test_dynamic_debug/parameters/p_level_num
> > +
> > + local hash_sysfs=$(slice_and_hash_ddctrl '\[test_dynamic_debug\]')
>
> [Severity: Medium]
> Does this classmap inheritance hash check exclude the submodule?
>
> Because of the strict regex using escaped brackets, the grep strictly matches
> the parent module's section and filters out all [test_dynamic_debug_submod]
> lines.
>
> Consequently, the hash equivalence check only proves the parent module
> responds
> correctly, ignoring the dependent submodule whose inheritance is supposedly
> being tested in FT_classmap_inheritance.
>
The test does not ignore the submodule; earlier steps in
FT_classmap_inheritance explicitly verify submodule inheritance across
both modules via verify_control_slice 'test_dynamic_debug' against
the golden records database, and step 6 validates runtime logging.
However, broadening the step 5 modprobe-vs-sysfs equivalence check
to verify both modules simultaneously is a good improvement. For v12,
I've updated the pattern to '\[test_dynamic_debug*\]', proving that
both supermod and submod callsites match identically across load-time
and runtime parameter configuration.
> > + if [ "$hash_modprobe" != "$hash_sysfs" ]; then
> > + echo -e "${RED}: Load-time vs runtime parameter equivalence check
> > failed!${NC}"
> > + exit $ksft_fail
> > + else
> > + v_echo "${GREEN}: Proven: parameter load-time (modprobe) " \
> > + "and runtime (sysfs write) are equivalent!${NC}"
> > + fi
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/[email protected]?part=24
