Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] selftests: test flakiness from unfiltered dmesg temporal slicing - [Medium] selftests: test failure coupled to unrelated kernel/params.c pr_debug churn --
commit 56bb89808ed29ee403a2b674106329de27cb9b06 Author: Jim Cromie <[email protected]> selftests/dyndbg: Add kselftest script to verify dynamic-debug This commit introduces a dynamic-debug selftest script and a reusable helper library for baseline-change-verify testing. It uses spatial control-file slicing and temporal syslog slicing to verify valid syntax, error handling, and basic queries against the built-in kernel/params.c file. > diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh > b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh > new file mode 100755 > index 0000000000000..c1475c7555429 > --- /dev/null > +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh [ ... ] > +function FT_basic_queries { > + v_echo "${GREEN}# BASIC_TESTS ${NC}" > + if [ $LACK_DD_BUILTIN -eq 1 ]; then > + echo "SKIP - test requires dynamic_debug built into kernel" > + return > + fi > + local f='kernel/params.c' > + ddcmd =_ # zero everything > + > + ddcmd "file $f +mf" "$f" [Severity: Medium] By strictly validating the exact state of kernel/params.c and hashing the normalized text, won't this test break if an unrelated upstream commit legitimately adds or removes a pr_debug() in kernel/params.c? Does this inadvertently couple the selftest to unrelated core kernel changes? > + ddcmd "file $f +l" "$f" > + ddcmd "file $f -m" "$f" > + ddcmd "file $f =_" "$f" [ ... ] > diff --git a/tools/testing/selftests/dynamic_debug/syslog_hash_validation.sh > b/tools/testing/selftests/dynamic_debug/syslog_hash_validation.sh > new file mode 100644 > index 0000000000000..8c6e91f5d9c27 > --- /dev/null > +++ b/tools/testing/selftests/dynamic_debug/syslog_hash_validation.sh [ ... ] > +function verify_dmesg_slice { > + # Slices dmesg, computes its hash, and verifies it against the database. > + # $1 - unique test key (e.g. normal_513) > + # $2 - optional start marker (defaults to ${APP}_START_${label}) > + # $3 - optional end marker (defaults to ${APP}_END_${label}) > + > + local label="$1" > + local app="${APP:-DYNDBG}" > + local start_marker="${2:-${app}_START_${label}_$$}" > + local end_marker="${3:-${app}_END_${label}_$$}" > + > + # 1. Capture the log slice (exactly once!) > + local log_slice=$(dmesg | sed -n "/$start_marker/,/$end_marker/p" | \ > + grep -E -v "$start_marker|$end_marker" | \ > + sed -E -e 's/^(\[[^]]*\][[:space:]]*)+//' ) [Severity: Medium] Since verify_dmesg_slice captures all kernel logs between test markers and hashes the entire block without filtering for test-specific messages, could concurrent unrelated kernel subsystems (like networking or RCU) emit printk logs that alter the MD5 hash and cause false positive test failures? > + > + # 2. Compute its fingerprint > + local fingerprint=$(echo "$log_slice" | tr -d '\r' | md5sum | cut -d' ' > -f1) > + > + # 3. Verify > + verify_fingerprint "$label" "$fingerprint" "Dmesg Log" "$log_slice" > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
