Message-ID: <a9d025a6-a880-469b-9863-aab104260122@bootlin.com>
Date: Tue, 15 Apr 2025 12:02:07 +0200
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Jim Cromie <jim.cromie@...il.com>, jbaron@...mai.com,
 gregkh@...uxfoundation.org, ukaszb@...omium.org, linux-kernel@...r.kernel.org
Cc: dri-devel@...ts.freedesktop.org, amd-gfx@...ts.freedesktop.org,
 intel-gvt-dev@...ts.freedesktop.org, intel-gfx@...ts.freedesktop.org,
 daniel.vetter@...ll.ch, tvrtko.ursulin@...ux.intel.com,
 jani.nikula@...el.com, ville.syrjala@...ux.intel.com
Subject: Re: [PATCH v3 18/54] selftests-dyndbg: add
 tools/testing/selftests/dynamic_debug/*



Le 02/04/2025 à 19:41, Jim Cromie a écrit :
> Add a selftest script for dynamic-debug.  The config requires
> CONFIG_TEST_DYNAMIC_DEBUG=m and CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=m,
> which tacitly requires either CONFIG_DYNAMIC_DEBUG=y or
> CONFIG_DYNAMIC_DEBUG_CORE=y
> 
> ATM this has just basic_tests(), which modify pr_debug() flags in the
> builtin params module.  This means they're available to manipulate and
> observe the effects in "cat control".
> 
> This is backported from another feature branch; the support-fns (thx
> Lukas) have unused features at the moment, they'll get used shortly.
> 
> The script enables simple virtme-ng testing:
> 
>     [jimc@...dalf b0-ftrace]$ vrun_t
>     virtme-ng 1.32+115.g07b109d
>     doing: vng --name v6.14-rc4-60-gd5f48427de0c \
>     	  --user root -v -p 4 -a dynamic_debug.verbose=3 V=1 \
> 	  -- ../tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
>     virtme: waiting for virtiofsd to start
>     ..
> 
> And add dynamic_debug to TARGETS, so `make run_tests` sees it properly
> 
> For the impatient, set TARGETS explicitly:
> 
>    bash-5.2# make TARGETS=dynamic_debug run_tests
>    make[1]: ...
>    TAP version 13
>    1..1
>    [   35.552922] dyndbg: read 3 bytes from userspace
>    [   35.553099] dyndbg: query 0: "=_" mod:*
>    [   35.553544] dyndbg: processed 1 queries, with 1778 matches, 0 errs
> 
> Signed-off-by: Jim Cromie <jim.cromie@...il.com>
> Co-developed-by: Łukasz Bartosik <ukaszb@...omium.org>
> Signed-off-by: Łukasz Bartosik <ukaszb@...omium.org>
> ---
> -r3 turn off green at end
>      drop config dep on TEST_DYNAMIC_DEBUG,
>      since basic-test uses builtin params
> 
> - check KCONFIG_CONFIG to avoid silly fails
> 
> Several tests are dependent upon config choices. Lets avoid failing
> where that is noise.
> 
> The KCONFIG_CONFIG var exists to convey the config-file around.  If
> the var names a file, read it and extract the relevant CONFIG items,
> and use them to skip the dependent tests, thus avoiding the fails that
> would follow, and the disruption to whatever CI is running these
> selftests.
> 
> If the envar doesn't name a config-file, ".config" is assumed.
> 
> CONFIG_DYNAMIC_DEBUG=y:
> 
> basic-tests() and comma-terminator-tests() test for the presence of
> the builtin pr_debugs in module/main.c, which I deemed stable and
> therefore safe to count.  That said, the test fails if only
> CONFIG_DYNAMIC_DEBUG_CORE=y is set.  It could be rewritten to test
> against test-dynamic-debug.ko, but that just trades one config
> dependence for another.
> 
> CONFIG_TEST_DYNAMIC_DEBUG=m
> 
> As written, test_percent_splitting() modprobes test_dynamic_debug,
> enables several classes, and count them.  It could be re-written to
> work for the builtin module also, but builtin test modules are not a
> common or desirable build/config.
> 
> CONFIG_TEST_DYNAMIC_DEBUG=m && CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=m
> 
> test_mod_submod() recaps the bug found in DRM-CI where drivers werent
> enabled by drm.debug=<bits>.  It modprobes both test_dynamic_debug &
> test_dynamic_debug_submod, so it depends on a loadable modules config.
> 
> It could be rewritten to work in a builtin parent config; DRM=y is
> common enough to be pertinent, but testing that config also wouldn't
> really test anything more fully than all-loadable modules, since they
> default together.
> 
> generalize-test-env
> ---
>   MAINTAINERS                                   |   1 +
>   tools/testing/selftests/Makefile              |   1 +
>   .../testing/selftests/dynamic_debug/Makefile  |   9 +
>   tools/testing/selftests/dynamic_debug/config  |   7 +
>   .../dynamic_debug/dyndbg_selftest.sh          | 257 ++++++++++++++++++
>   5 files changed, 275 insertions(+)
>   create mode 100644 tools/testing/selftests/dynamic_debug/Makefile
>   create mode 100644 tools/testing/selftests/dynamic_debug/config
>   create mode 100755 tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1c5fcbd9e408..1192ad6c65c1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8140,6 +8140,7 @@ S:	Maintained
>   F:	include/linux/dynamic_debug.h
>   F:	lib/dynamic_debug.c
>   F:	lib/test_dynamic_debug*.c
> +F:	tools/testing/selftests/dynamic_debug/*
>   
>   DYNAMIC INTERRUPT MODERATION
>   M:	Tal Gilboa <talgi@...dia.com>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 8daac70c2f9d..b6a323c7f986 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -26,6 +26,7 @@ TARGETS += drivers/net/team
>   TARGETS += drivers/net/virtio_net
>   TARGETS += drivers/platform/x86/intel/ifs
>   TARGETS += dt
> +TARGETS += dynamic_debug
>   TARGETS += efivarfs
>   TARGETS += exec
>   TARGETS += fchmodat2
> diff --git a/tools/testing/selftests/dynamic_debug/Makefile b/tools/testing/selftests/dynamic_debug/Makefile
> new file mode 100644
> index 000000000000..6d06fa7f1040
> --- /dev/null
> +++ b/tools/testing/selftests/dynamic_debug/Makefile
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +# borrowed from Makefile for user memory selftests
> +
> +# No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
> +all:
> +
> +TEST_PROGS := dyndbg_selftest.sh
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/dynamic_debug/config b/tools/testing/selftests/dynamic_debug/config
> new file mode 100644
> index 000000000000..0f906ff53908
> --- /dev/null
> +++ b/tools/testing/selftests/dynamic_debug/config
> @@ -0,0 +1,7 @@
> +
> +# basic tests ref the builtin params module
> +CONFIG_DYNAMIC_DEBUG=m
> +
> +# more testing is possible with these
> +# CONFIG_TEST_DYNAMIC_DEBUG=m
> +# CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=m
> 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 000000000000..465fad3f392c
> --- /dev/null
> +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> @@ -0,0 +1,257 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +V=${V:=0}  # invoke as V=1 $0  for global verbose
> +RED="\033[0;31m"
> +GREEN="\033[0;32m"
> +YELLOW="\033[0;33m"
> +BLUE="\033[0;34m"
> +MAGENTA="\033[0;35m"
> +CYAN="\033[0;36m"
> +NC="\033[0;0m"
> +error_msg=""
> +
> +[ -e /proc/dynamic_debug/control ] || {
> +    echo -e "${RED}: this test requires CONFIG_DYNAMIC_DEBUG=y ${NC}"
> +    exit 0 # nothing to test here, no good reason to fail.
> +}
> +
> +# need info to avoid failures due to untestable configs
> +
> +[ -f "$KCONFIG_CONFIG" ] || KCONFIG_CONFIG=".config"
> +if [ -f "$KCONFIG_CONFIG" ]; then
> +    echo "# consulting KCONFIG_CONFIG: $KCONFIG_CONFIG"
> +    grep -q "CONFIG_DYNAMIC_DEBUG=y" $KCONFIG_CONFIG ; LACK_DD_BUILTIN=$?
> +    grep -q "CONFIG_TEST_DYNAMIC_DEBUG=m" $KCONFIG_CONFIG ; LACK_TMOD=$?
> +    grep -q "CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=m" $KCONFIG_CONFIG ; LACK_TMOD_SUBMOD=$?
> +    if [ $V -eq 1 ]; then
> +	echo LACK_DD_BUILTIN: $LACK_DD_BUILTIN
> +	echo LACK_TMOD: $LACK_TMOD
> +	echo LACK_TMOD_SUBMOD: $LACK_TMOD_SUBMOD
> +    fi
> +else
> +    LACK_DD_BUILTIN=0
> +    LACK_TMOD=0
> +    LACK_TMOD_SUBMOD=0
> +fi

Nitpick for the sh file: is it normal to have inconsistent indenting ?(4 
space, tabs, 8 spaces)

> +function vx () {
> +    echo $1 > /sys/module/dynamic_debug/parameters/verbose
> +}
> +
> +function ddgrep () {
> +    grep $1 /proc/dynamic_debug/control
> +}
> +
> +function doprints () {
> +    cat /sys/module/test_dynamic_debug/parameters/do_prints
> +}
> +
> +function ddcmd () {
> +    exp_exit_code=0
> +    num_args=$#
> +    if [ "${@:$#}" = "pass" ]; then
> +	num_args=$#-1
> +    elif [ "${@:$#}" = "fail" ]; then
> +        num_args=$#-1
> +	exp_exit_code=1
> +    fi
> +    args=${@:1:$num_args}
> +    output=$((echo "$args" > /proc/dynamic_debug/control) 2>&1)
> +    exit_code=$?
> +    error_msg=$(echo $output | cut -d ":" -f 5 | sed -e 's/^[[:space:]]*//')
> +    handle_exit_code $BASH_LINENO $FUNCNAME $exit_code $exp_exit_code
> +}
> +
> +function handle_exit_code() {
> +    local exp_exit_code=0
> +    [ $# == 4 ] && exp_exit_code=$4
> +    if [ $3 -ne $exp_exit_code ]; then
> +        echo -e "${RED}: $BASH_SOURCE:$1 $2() expected to exit with code $exp_exit_code"
> +	[ $3 == 1 ] && echo "Error: '$error_msg'"
> +        exit
> +    fi
> +}
> +
> +# $1 - pattern to match, pattern in $1 is enclosed by spaces for a match ""\s$1\s"
> +# $2 - number of times the pattern passed in $1 is expected to match
> +# $3 - optional can be set either to "-r" or "-v"
> +#       "-r" means relaxed matching in this case pattern provided in $1 is passed
> +#       as is without enclosing it with spaces
> +#       "-v" prints matching lines
> +# $4 - optional when $3 is set to "-r" then $4 can be used to pass "-v"
> +function check_match_ct {
> +    pattern="\s$1\s"
> +    exp_cnt=0
> +
> +    [ "$3" == "-r" ] && pattern="$1"
> +    let cnt=$(ddgrep "$pattern" | wc -l)
> +    if [ $V -eq 1 ] || [ "$3" == "-v" ] || [ "$4" == "-v" ]; then
> +        echo -ne "${BLUE}" && ddgrep $pattern && echo -ne "${NC}"
> +    fi
> +    [ $# -gt 1 ] && exp_cnt=$2
> +    if [ $cnt -ne $exp_cnt ]; then
> +        echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO check failed expected $exp_cnt on $1, got $cnt"
> +        exit
> +    else
> +        echo ": $cnt matches on $1"
> +    fi
> +}
> +
> +# $1 - trace instance name
> +# #2 - if > 0 then directory is expected to exist, if <= 0 then otherwise
> +# $3 - "-v" for verbose
> +function check_trace_instance_dir {
> +    if [ -e /sys/kernel/tracing/instances/$1 ]; then
> +        if [ "$3" == "-v" ] ; then
> +            echo "ls -l /sys/kernel/tracing/instances/$1: "
> +            ls -l /sys/kernel/tracing/instances/$1
> +        fi
> +	if [ $2 -le 0 ]; then
> +            echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error trace instance \
> +		    '/sys/kernel/tracing/instances/$1' does exist"
> +	    exit
> +	fi
> +    else
> +	if [ $2 -gt 0 ]; then
> +            echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error trace instance \
> +		    '/sys/kernel/tracing/instances/$1' does not exist"
> +	    exit
> +        fi
> +    fi
> +}
> +
> +function tmark {
> +    echo $* > /sys/kernel/tracing/trace_marker
> +}
> +
> +# $1 - trace instance name
> +# $2 - line number
> +# $3 - if > 0 then the instance is expected to be opened, otherwise
> +# the instance is expected to be closed
> +function check_trace_instance {
> +    output=$(tail -n9 /proc/dynamic_debug/control | grep ": Opened trace instances" \
> +	    | xargs -n1 | grep $1)
> +    if [ "$output" != $1 ] && [ $3 -gt 0 ]; then
> +        echo -e "${RED}: $BASH_SOURCE:$2 trace instance $1 is not opened"
> +        exit
> +    fi
> +    if [ "$output" == $1 ] && [ $3 -le 0 ]; then
> +        echo -e "${RED}: $BASH_SOURCE:$2 trace instance $1 is not closed"
> +        exit
> +    fi
> +}
> +
> +function is_trace_instance_opened {
> +    check_trace_instance $1 $BASH_LINENO 1
> +}
> +
> +function is_trace_instance_closed {
> +    check_trace_instance $1 $BASH_LINENO 0
> +}
> +
> +# $1 - trace instance directory to delete
> +# $2 - if > 0 then directory is expected to be deleted successfully, if <= 0 then otherwise
> +function del_trace_instance_dir() {
> +    exp_exit_code=1
> +    [ $2 -gt 0 ] && exp_exit_code=0
> +    output=$((rmdir /sys/kernel/debug/tracing/instances/$1) 2>&1)
> +    exit_code=$?
> +    error_msg=$(echo $output | cut -d ":" -f 3 | sed -e 's/^[[:space:]]*//')
> +    handle_exit_code $BASH_LINENO $FUNCNAME $exit_code $exp_exit_code
> +}
> +
> +function error_log_ref {
> +    # to show what I got
> +    : echo "# error-log-ref: $1"
> +    : echo cat \$2
> +}
> +
> +function ifrmmod {
> +    lsmod | grep $1 2>&1>/dev/null && rmmod $1
> +}
> +
> +# $1 - text to search for
> +function search_trace() {
> +    search_trace_name 0 1 $1
> +}
> +
> +# $1 - trace instance name, 0 for global event trace
> +# $2 - line number counting from the bottom
> +# $3 - text to search for
> +function search_trace_name() {
> +	if [ "$1" = "0" ]; then
> +	    buf=$(cat /sys/kernel/debug/tracing/trace)
> +	    line=$(tail -$2 /sys/kernel/debug/tracing/trace | head -1 | sed -e 's/^[[:space:]]*//')
> +	else
> +	    buf=$(cat /sys/kernel/debug/tracing/instances/$1/trace)
> +	    line=$(tail -$2 /sys/kernel/debug/tracing/instances/$1/trace | head -1 | \
> +		   sed -e 's/^[[:space:]]*//')
> +	fi
> +	if [ $2 = 0 ]; then
> +	    # whole-buf check
> +	    output=$(echo $buf | grep "$3")
> +	else
> +	    output=$(echo $line | grep "$3")
> +	fi
> +	if [ "$output" = "" ]; then
> +            echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO search for '$3' failed \
> +		    in line '$line' or '$buf'"
> +	    exit
> +	fi
> +	if [ $V = 1 ]; then
> +	    echo -e "${MAGENTA}: search_trace_name in $1 found: \n$output \nin:${BLUE} $buf ${NC}"
> +        fi
> +}
> +
> +# $1 - error message to check
> +function check_err_msg() {
> +    if [ "$error_msg" != "$1" ]; then
> +        echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error message '$error_msg' \
> +		does not match with '$1'"
> +        exit
> +    fi
> +}
> +
> +function basic_tests {
> +    echo -e "${GREEN}# BASIC_TESTS ${NC}"
> +    if [ $LACK_DD_BUILTIN -eq 1 ]; then
> +	echo "SKIP"
> +	return
> +    fi
> +    ddcmd =_ # zero everything
> +    check_match_ct =p 0
> +
> +    # module params are builtin to handle boot args
> +    check_match_ct '\[params\]' 4 -r
> +    ddcmd module params +mpf
> +    check_match_ct =pmf 4
> +
> +    # multi-cmd input, newline separated, with embedded comments
> +    cat <<"EOF" > /proc/dynamic_debug/control
> +      module params =_				# clear params
> +      module params +mf				# set flags
> +      module params func parse_args +sl		# other flags
> +EOF
> +    check_match_ct =mf 3
> +    check_match_ct =mfsl 1
> +    ddcmd =_
> +}
> +
> +tests_list=(
> +    basic_tests
> +)
> +
> +# Run tests
> +
> +ifrmmod test_dynamic_debug_submod
> +ifrmmod test_dynamic_debug
> +
> +for test in "${tests_list[@]}"
> +do
> +    $test
> +    echo ""
> +done
> +echo -en "${GREEN}# Done on: "
> +date
> +echo -en "${NC}"

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Powered by blists - more mailing lists