Re: [PATCH v17 00/14] Add support for MT8195 SCP 2nd core
> Other than patch 2 and 14, I have applied this set. The remaining patches > will > have to be resent to Matthias. > Thanks, > Mathieu Hello, With patch 2 missing, the SCP is not probed correctly anymore on asurada (MT8192) and kukui (MT8183). The mtk-scp driver relies on the existence of the `cros-ec-rpmsg` node in the dt to determine if the SCP is single or multicore. Without patch 2 the driver wrongly assumes the SCP on MT8192 and MT8183 are multicore, leading to the following errors during initialization: 10696 04:33:59.126671 <3>[ 15.465714] platform 1050.scp:cros-ec: invalid resource (null) 10697 04:33:59.142855 <3>[ 15.478560] platform 1050.scp:cros-ec: Failed to parse and map sram memory 10698 04:33:59.149650 <3>[ 15.486121] mtk-scp 1050.scp: Failed to initialize core 0 rproc The issue was caught by KernelCI, complete logs can be found here: - asurada: https://storage.kernelci.org/next/master/next-20230914/arm64/defconfig+arm64-chromebook+videodec/gcc-10/lab-collabora/baseline-nfs-mt8192-asurada-spherion-r0.html - kukui: https://storage.kernelci.org/next/master/next-20230914/arm64/defconfig+arm64-chromebook+videodec/gcc-10/lab-collabora/baseline-nfs-mt8183-kukui-jacuzzi-juniper-sku16.html Reporting the issue so that patch 2 and 14 can be resent and merged soon. Best, Laura
Re: [PATCH v17 00/14] Add support for MT8195 SCP 2nd core
On 9/19/23 03:14, Mathieu Poirier wrote: > On Mon, Sep 18, 2023 at 12:31:41PM +0200, Laura Nao wrote: >>> Other than patch 2 and 14, I have applied this set. The remaining patches >>> will >>> have to be resent to Matthias. >> >>> Thanks, >>> Mathieu >> >> Hello, >> >> With patch 2 missing, the SCP is not probed correctly anymore on asurada >> (MT8192) and kukui (MT8183). The mtk-scp driver relies on the existence of >> the `cros-ec-rpmsg` node in the dt to determine if the SCP is single or >> multicore. Without patch 2 the driver wrongly assumes the SCP on MT8192 and >> MT8183 are multicore, leading to the following errors during initialization: >> >> 10696 04:33:59.126671 <3>[ 15.465714] platform 1050.scp:cros-ec: >> invalid resource (null) >> 10697 04:33:59.142855 <3>[ 15.478560] platform 1050.scp:cros-ec: >> Failed to parse and map sram memory >> 10698 04:33:59.149650 <3>[ 15.486121] mtk-scp 1050.scp: Failed to >> initialize core 0 rproc >> >> The issue was caught by KernelCI, complete logs can be found here: >> - asurada: >> https://storage.kernelci.org/next/master/next-20230914/arm64/defconfig+arm64-chromebook+videodec/gcc-10/lab-collabora/baseline-nfs-mt8192-asurada-spherion-r0.html >> - kukui: >> https://storage.kernelci.org/next/master/next-20230914/arm64/defconfig+arm64-chromebook+videodec/gcc-10/lab-collabora/baseline-nfs-mt8183-kukui-jacuzzi-juniper-sku16.html >> >> Reporting the issue so that patch 2 and 14 can be resent and merged soon. >> > > Apologies for the trouble here, the error is mine. > > I have applied and pushed patch 02 - please confirm that things are working as > expected now. Matthias will need to either ack patch 14 or pick it up > himself. > > I confirm SCP is probed correctly on MT8192 (spherion) and MT8183 (juniper) on the remoteproc tree (for-next branch) now. Thank you!
Re: [PATCH] remoteproc: mediatek: Refactor single core check and fix retrocompatibility
On 9/19/23 11:23, AngeloGioacchino Del Regno wrote:
> In older devicetrees we had the ChromeOS EC in a node called "cros-ec"
> instead of the newer "cros-ec-rpmsg", but this driver is now checking
> only for the latter, breaking compatibility with those.
>
> Besides, we can check if the SCP is single or dual core by simply
> walking through the children of the main SCP node and checking if
> if there's more than one "mediatek,scp-core" compatible node.
>
> Fixes: 1fdbf0cdde98 ("remoteproc: mediatek: Probe SCP cluster on multi-core
> SCP")
> Signed-off-by: AngeloGioacchino Del Regno
>
> ---
> drivers/remoteproc/mtk_scp.c | 18 +++---
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
Tested on asurada (spherion) and jacuzzi (juniper). The issue was detected by
KernelCI, so:
Reported-by: "kernelci.org bot"
Tested-by: Laura Nao
Thanks!
Laura
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index ea227b566c54..a35409eda0cf 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -1144,29 +1144,25 @@ static int scp_add_multi_core(struct platform_device
> *pdev,
> return ret;
> }
>
> -static int scp_is_single_core(struct platform_device *pdev)
> +static bool scp_is_single_core(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct device_node *np = dev_of_node(dev);
> struct device_node *child;
> + int num_cores = 0;
>
> - child = of_get_next_available_child(np, NULL);
> - if (!child)
> - return dev_err_probe(dev, -ENODEV, "No child node\n");
> + for_each_child_of_node(np, child)
> + if (of_device_is_compatible(child, "mediatek,scp-core"))
> + num_cores++;
>
> - of_node_put(child);
> - return of_node_name_eq(child, "cros-ec-rpmsg");
> + return num_cores < 2;
> }
>
> static int scp_cluster_init(struct platform_device *pdev, struct
> mtk_scp_of_cluster *scp_cluster)
> {
> int ret;
>
> - ret = scp_is_single_core(pdev);
> - if (ret < 0)
> - return ret;
> -
> - if (ret)
> + if (scp_is_single_core(pdev))
> ret = scp_add_single_core(pdev, scp_cluster);
> else
> ret = scp_add_multi_core(pdev, scp_cluster);
Testing Quality Call notes - 2024-11-03
Hello, KernelCI is hosting a bi-weekly call on Thursday to discuss improvements to existing upstream tests, the development of new tests to increase kernel testing coverage, and the enablement of these tests in KernelCI. Below is a list of the tests the community has been working on and their latest status updates, as discussed in the last meeting held on 2024-11-03: *Missing devices kselftest* - Proposing new kselftest to report devices that go missing in the system (v2): https://lore.kernel.org/all/20240928-kselftest-dev-exist-v2-1-fab07de6b...@collabora.com/ - Sent v2 addressing feedback received on the RFCv1 and during the session at LPC 2024: https://www.youtube.com/live/kcr8NXEbzcg?si=QWBvJAOjj7tg264o&t=11283 *Boot time test* - RFC: https://lore.kernel.org/all/20240725110622.96301-1-laura@collabora.com/T/#ma568382acdc81af65c30fe3823a7be3e49f98108 - Discussed proposal at LPC2024: https://www.youtube.com/live/8XQwzUZxLK4?feature=shared&t=16944 - Planning on preparing v2, based on feedback received in the session - Suggestions for improvements and additional features include: exploring bootloader tracing via pre-filled ftrace buffers, adding support for specifying variance values on a per-event basis, investigating the use of ftrace histograms for initcalls *Device testing documentation* - Patch: https://lore.kernel.org/all/20241001-kselftest-device-docs-v1-1-be28b70dd...@collabora.com/ - Submitted documentation on device testing, detailing the types of kselftests available, their requirements, and the coverage they provide. The goal is to guide users in selecting the appropriate tests for their devices. *GPIO test* - RFC: https://lore.kernel.org/all/20240909-kselftest-gpio-set-get-config-v1-0-16a065afc...@collabora.com/ - Proposed a new kselftest to verify the GPIO driver functionality. The test uses a YAML-based test plan that specifies the configurations to be checked. It sets each pin configuration and retrieves it to ensure they match. Currently, the test only verifies bias settings, but it can be easily extended to cover additional pin configurations. Please reply to this thread if you'd like to join the call or discuss any of the topics further. We look forward to collaborating with the community to improve upstream tests and expand coverage to more areas of interest within the kernel. Best regards, Laura Nao
[PATCH v2] kselftests: Add test to detect boot event slowdowns
Introduce a new kselftest to identify slowdowns in key boot events. This test uses ftrace to monitor the start and end times, as well as the durations of all initcalls, and compares these timings to reference values to identify significant slowdowns. The script functions in two modes: the 'generate' mode allows to create a JSON file containing initial reference timings for all initcalls from a known stable kernel. The 'test' mode can be used during subsequent boots to assess current timings against the reference values and determine if there are any significant differences. The test ships with a bootconfig file for setting up ftrace and a configuration fragment for the necessary kernel configs. Signed-off-by: Laura Nao --- Hello, This v2 is a follow-up to RFCv1[1] and includes changes based on feedback from the LPC 2024 session [2], along with some other fixes. [1] https://lore.kernel.org/all/20240725110622.96301-1-laura@collabora.com/ [2] https://www.youtube.com/watch?v=rWhW2-Vzi40 After reviewing other available tests and considering the feedback from discussions at Plumbers, I decided to stick with the bootconfig file approach but extend it to track all initcalls instead of a fixed set of functions or events. The bootconfig file can be expanded and adapted to track additional functions if needed for specific use cases. I also defined a synthetic event to calculate initcall durations, while still tracking their start and end times. Users are then allowed to choose whether to compare start times, end times, or durations. Support for specifying different rules for different initcalls has also been added. In RFCv1, there was some discussion about using existing tools like bootgraph.py. However, the output from these tools is mainly for manual inspection (e.g., HTML visual output), whereas this test is designed to run in automated CI environments too. The kselftest proposed here combines the process of generating reference data and running tests into a single script with two modes, making it easy to integrate into automated workflows. Many of the features in this v2 (e.g., generating a JSON reference file, comparing timings, and reporting results in KTAP format) could potentially be integrated into bootgraph.py with some effort. However, since this test is intended for automated execution rather than manual use, I've decided to keep it separate for now and explore the options suggested at LPC, such as using ftrace histograms for initcall latencies. I'm open to revisiting this decision and working toward integrating the changes into bootgraph.py if there's a strong preference for unifying the tools. Let me know your thoughts. A comprehensive changelog is reported below. Thanks, Laura --- Changes in v2: - Updated ftrace configuration to track all initcall start times, end times, and durations, and generate a histogram. - Modified test logic to compare initcall durations by default, with the option to compare start or end times if needed. - Added warnings if the initcalls in the reference file differ from those detected in the running system. - Combined the scripts into a single script with two modes: one for generating the reference file and one for running the test. - Added support for specifying different rules for individual initcalls. - Switched the reference format from YAML to JSON. - Added metadata to the reference file, including kernel version, kernel configuration, and cmdline. - Link to v1: https://lore.kernel.org/all/20240725110622.96301-1-laura@collabora.com/ --- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/boot-time/Makefile| 16 ++ tools/testing/selftests/boot-time/bootconfig | 15 + tools/testing/selftests/boot-time/config | 6 + .../selftests/boot-time/test_boot_time.py | 265 ++ 5 files changed, 303 insertions(+) create mode 100644 tools/testing/selftests/boot-time/Makefile create mode 100644 tools/testing/selftests/boot-time/bootconfig create mode 100644 tools/testing/selftests/boot-time/config create mode 100755 tools/testing/selftests/boot-time/test_boot_time.py diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index b38199965f99..1bb20d1e3854 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -3,6 +3,7 @@ TARGETS += acct TARGETS += alsa TARGETS += amd-pstate TARGETS += arm64 +TARGETS += boot-time TARGETS += bpf TARGETS += breakpoints TARGETS += cachestat diff --git a/tools/testing/selftests/boot-time/Makefile b/tools/testing/selftests/boot-time/Makefile new file mode 100644 index ..cdcdc1bbe779 --- /dev/null +++ b/tools/testing/selftests/boot-time/Makefile @@ -0,0 +1,16 @@ +PY3 = $(shell which python3 2>/dev/null) + +ifneq ($(PY3),) + +TEST_PROGS := test_boot_time.py + +include ../lib.mk + +else + +all: no_py3_warning + +no_py3_warning: + @echo "
Testing Quality Call notes - 2024-10-31
Hello, KernelCI is hosting a bi-weekly call on Thursday to discuss improvements to existing upstream tests, the development of new tests to increase kernel testing coverage, and the enablement of these tests in KernelCI. In recent months, we at Collabora have focused on various kernel areas, assessing the tests already available upstream and contributing patches to make them easily runnable in CIs. Below is a list of the tests we've been working on and their latest status updates, as discussed in the last meeting held on 2024-10-31: *Boot time test* - Sent v2: https://lore.kernel.org/all/20241018101439.20849-1-laura@collabora.com/ - Changes in v2 driven by feedback received in the LPC session *ACPI probe kselftest* - RFCv2: https://lore.kernel.org/all/20240308144933.337107-1-laura@collabora.com/ - Related upstream discussion on probe status tracking through printks: https://lore.kernel.org/lkml/dc7af563-530d-4e1f-bcbb-90bcfc2fe11a@stanley.mountain/ Please reply to this thread if you'd like to join the call or discuss any of the topics further. We look forward to collaborating with the community to improve upstream tests and expand coverage to more areas of interest within the kernel. Best regards, Laura Nao
[PATCH v2 1/2] selftests/watchdog: add -c option to limit the ping loop
In order to run the watchdog selftest in a non-interactive environment,
the loop responsible for pinging the watchdog should be finite.
Introduce a new '-c' option to adjust the number of pings as needed.
Signed-off-by: Laura Nao
---
tools/testing/selftests/watchdog/watchdog-test.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/watchdog/watchdog-test.c
b/tools/testing/selftests/watchdog/watchdog-test.c
index bc71cbca0dde..58c25015d5e7 100644
--- a/tools/testing/selftests/watchdog/watchdog-test.c
+++ b/tools/testing/selftests/watchdog/watchdog-test.c
@@ -27,13 +27,14 @@
int fd;
const char v = 'V';
-static const char sopts[] = "bdehp:st:Tn:NLf:i";
+static const char sopts[] = "bdehp:c:st:Tn:NLf:i";
static const struct option lopts[] = {
{"bootstatus", no_argument, NULL, 'b'},
{"disable", no_argument, NULL, 'd'},
{"enable", no_argument, NULL, 'e'},
{"help",no_argument, NULL, 'h'},
{"pingrate", required_argument, NULL, 'p'},
+ {"pingcount", required_argument, NULL, 'c'},
{"status", no_argument, NULL, 's'},
{"timeout", required_argument, NULL, 't'},
{"gettimeout", no_argument, NULL, 'T'},
@@ -90,6 +91,7 @@ static void usage(char *progname)
printf(" -h, --help\t\tPrint the help message\n");
printf(" -p, --pingrate=P\tSet ping rate to P seconds (default %d)\n",
DEFAULT_PING_RATE);
+ printf(" -c, --pingcount=C\tLimit the number of pings to C (default
infinite)\n");
printf(" -t, --timeout=T\tSet timeout to T seconds\n");
printf(" -T, --gettimeout\tGet the timeout\n");
printf(" -n, --pretimeout=T\tSet the pretimeout to T seconds\n");
@@ -172,6 +174,7 @@ int main(int argc, char *argv[])
{
int flags;
unsigned int ping_rate = DEFAULT_PING_RATE;
+ unsigned int ping_count = -1;
int ret;
int c;
int oneshot = 0;
@@ -248,6 +251,12 @@ int main(int argc, char *argv[])
ping_rate = DEFAULT_PING_RATE;
printf("Watchdog ping rate set to %u seconds.\n",
ping_rate);
break;
+ case 'c':
+ ping_count = strtoul(optarg, NULL, 0);
+ if (!ping_count)
+ oneshot = 1;
+ printf("Number of pings set to %u.\n", ping_count);
+ break;
case 's':
flags = 0;
oneshot = 1;
@@ -336,9 +345,11 @@ int main(int argc, char *argv[])
signal(SIGINT, term);
- while (1) {
+ while (ping_count != 0) {
keep_alive();
sleep(ping_rate);
+ if (ping_count > 0)
+ ping_count--;
}
end:
/*
--
2.30.2
[PATCH v2 0/2] Modify the watchdog selftest for execution in non-interactive environments
This series is a follow-up to v1[1], aimed at making the watchdog selftest more suitable for CI environments. Currently, in non-interactive setups, the watchdog kselftest can only run with oneshot parameters, preventing the testing of the WDIOC_KEEPALIVE ioctl since the ping loop is only interrupted by SIGINT. The first patch adds a new -c option to limit the number of watchdog pings, allowing the test to be optionally finite. The second patch updates the test output to conform to KTAP. The default behavior remains unchanged: without the -c option, the keep_alive() loop continues indefinitely until interrupted by SIGINT. [1] https://lore.kernel.org/all/20240506111359.224579-1-laura@collabora.com/ Changes in v2: - The keep_alive() loop remains infinite by default - Introduced keep_alive_res variable to track the WDIOC_KEEPALIVE ioctl return code for user reporting Laura Nao (2): selftests/watchdog: add -c option to limit the ping loop selftests/watchdog: convert the test output to KTAP format .../selftests/watchdog/watchdog-test.c| 169 +++--- 1 file changed, 103 insertions(+), 66 deletions(-) -- 2.30.2
[PATCH v2 2/2] selftests/watchdog: convert the test output to KTAP format
Conform the test output to the KTAP format standard. The number of tests
executed is determined by the script arguments, and options such as
-c, -f, -h, -i, and -p do not impact the total test count.
Signed-off-by: Laura Nao
---
.../selftests/watchdog/watchdog-test.c| 158 ++
1 file changed, 92 insertions(+), 66 deletions(-)
diff --git a/tools/testing/selftests/watchdog/watchdog-test.c
b/tools/testing/selftests/watchdog/watchdog-test.c
index 58c25015d5e7..4781736070e3 100644
--- a/tools/testing/selftests/watchdog/watchdog-test.c
+++ b/tools/testing/selftests/watchdog/watchdog-test.c
@@ -22,12 +22,15 @@
#include
#include
#include
+#include "../kselftest.h"
#define DEFAULT_PING_RATE 1
int fd;
+int keep_alive_res;
const char v = 'V';
static const char sopts[] = "bdehp:c:st:Tn:NLf:i";
+static const char topts[] = "bdeLn:Nst:T";
static const struct option lopts[] = {
{"bootstatus", no_argument, NULL, 'b'},
{"disable", no_argument, NULL, 'd'},
@@ -51,7 +54,7 @@ static const struct option lopts[] = {
* the PC Watchdog card to reset its internal timer so it doesn't trigger
* a computer reset.
*/
-static void keep_alive(void)
+static int keep_alive(void)
{
int dummy;
int ret;
@@ -59,6 +62,8 @@ static void keep_alive(void)
ret = ioctl(fd, WDIOC_KEEPALIVE, &dummy);
if (!ret)
printf(".");
+
+ return ret;
}
/*
@@ -72,35 +77,36 @@ static void term(int sig)
close(fd);
if (ret < 0)
- printf("\nStopping watchdog ticks failed (%d)...\n", errno);
+ ksft_print_msg("\nStopping watchdog ticks failed (%d)...\n",
errno);
else
- printf("\nStopping watchdog ticks...\n");
- exit(0);
+ ksft_print_msg("\nStopping watchdog ticks...\n");
+ ksft_test_result(!keep_alive_res, "WDIOC_KEEPALIVE\n");
+ ksft_finished();
}
static void usage(char *progname)
{
- printf("Usage: %s [options]\n", progname);
- printf(" -f, --file\t\tOpen watchdog device file\n");
- printf("\t\t\tDefault is /dev/watchdog\n");
- printf(" -i, --info\t\tShow watchdog_info\n");
- printf(" -s, --status\t\tGet status & supported features\n");
- printf(" -b, --bootstatus\tGet last boot status (Watchdog/POR)\n");
- printf(" -d, --disable\t\tTurn off the watchdog timer\n");
- printf(" -e, --enable\t\tTurn on the watchdog timer\n");
- printf(" -h, --help\t\tPrint the help message\n");
- printf(" -p, --pingrate=P\tSet ping rate to P seconds (default %d)\n",
+ ksft_print_msg("Usage: %s [options]\n", progname);
+ ksft_print_msg(" -f, --file\t\tOpen watchdog device file\n");
+ ksft_print_msg("\t\t\tDefault is /dev/watchdog\n");
+ ksft_print_msg(" -i, --info\t\tShow watchdog_info\n");
+ ksft_print_msg(" -s, --status\t\tGet status & supported features\n");
+ ksft_print_msg(" -b, --bootstatus\tGet last boot status
(Watchdog/POR)\n");
+ ksft_print_msg(" -d, --disable\t\tTurn off the watchdog timer\n");
+ ksft_print_msg(" -e, --enable\t\tTurn on the watchdog timer\n");
+ ksft_print_msg(" -h, --help\t\tPrint the help message\n");
+ ksft_print_msg(" -p, --pingrate=P\tSet ping rate to P seconds (default
%d)\n",
DEFAULT_PING_RATE);
- printf(" -c, --pingcount=C\tLimit the number of pings to C (default
infinite)\n");
- printf(" -t, --timeout=T\tSet timeout to T seconds\n");
- printf(" -T, --gettimeout\tGet the timeout\n");
- printf(" -n, --pretimeout=T\tSet the pretimeout to T seconds\n");
- printf(" -N, --getpretimeout\tGet the pretimeout\n");
- printf(" -L, --gettimeleft\tGet the time left until timer expires\n");
- printf("\n");
- printf("Parameters are parsed left-to-right in real-time.\n");
- printf("Example: %s -d -t 10 -p 5 -e\n", progname);
- printf("Example: %s -t 12 -T -n 7 -N\n", progname);
+ ksft_print_msg(" -c, --pingcount=C\tSet number of pings to C (default
infinite)\n");
+ ksft_print_msg(" -t, --timeout=T\tSet timeout to T seconds\n");
+ ksft_print_msg(" -T, --gettimeout\tGet the timeout\n");
+ ksft_print_msg(" -n, --pretimeout=T\tSet the pretimeout to T
seconds\n");
+ ksft_print_msg(" -N, --getpretimeout\tGet the pretimeout\n");
+ ksft_print_msg(" -L, --get
Re: [PATCH] selftests: Warn about skipped tests in result summary
Hi Shuah and Usama,
On 11/25/24 09:46, Muhammad Usama Anjum wrote:
> Hi Laura,
>
> Thank you for making change.
>
> On 11/22/24 11:19 PM, Shuah wrote:
>> On 11/22/24 08:55, Laura Nao wrote:
>>> Update the functions that print the test totals at the end of a selftest
>>> to include a warning message when skipped tests are detected. The
>>> message advises users that skipped tests may indicate missing
>>> configuration options and suggests enabling them to improve coverage.
>>>
>>> Signed-off-by: Laura Nao
>>> ---
>>> This patch follows up on a previous discussion[1] and aims to highlight
>>> skipped tests for the user's attention.
>>>
>>> [1]
>>> https://lore.kernel.org/lkml/2bb2d338-cd00-4ac2-b8bd-5579eae82...@linuxfoundation.org/
>>> ---
>>>tools/testing/selftests/kselftest.h | 4
>>>tools/testing/selftests/kselftest/ksft.py | 3 +++
>>>tools/testing/selftests/kselftest/ktap_helpers.sh | 4
>>>3 files changed, 11 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/kselftest.h
>>> b/tools/testing/selftests/kselftest.h
>>> index 29fedf609611..d3f64b333acd 100644
>>> --- a/tools/testing/selftests/kselftest.h
>>> +++ b/tools/testing/selftests/kselftest.h
>>> @@ -147,6 +147,10 @@ static inline void ksft_set_plan(unsigned int plan)
>>> static inline void ksft_print_cnts(void)
>>>{
>>> +if (ksft_cnt.ksft_xskip > 0)
>>> +printf(
>>> +"# Skipped tests detected. Consider enabling relevant config
>>> options to improve coverage.\n"
> Looks good. Printing the number of skipped tests would be an improvement.
> I'm thinking about a case where some tests got failed and some skipped. Would
> this warning be useful in that case?
>
I believe the warning remains useful, as it helps users identify possible
gaps in their configuration - I think that's valuable regardless of the
results of other tests.
>>
>> I like this. How about printing the number of skipped tests in this
>> message also to make it easy to parse.
>>
>> Same comment on other print messages,
>>
Sure, that makes sense. I'll submit a v2 to include the number of skipped
tests.
>>> +);
>>>if (ksft_plan != ksft_test_num())
>>>printf("# Planned tests != run tests (%u != %u)\n",
>>>ksft_plan, ksft_test_num());
>>> diff --git a/tools/testing/selftests/kselftest/ksft.py
>>> b/tools/testing/selftests/kselftest/ksft.py
>>> index bf215790a89d..7675a15a1264 100644
>>> --- a/tools/testing/selftests/kselftest/ksft.py
>>> +++ b/tools/testing/selftests/kselftest/ksft.py
>>> @@ -27,6 +27,9 @@ def set_plan(num_tests):
>>>def print_cnts():
>>> +if ksft_cnt['skip'] > 0:
>>> +print("# Skipped tests detected. Consider enabling relevant config
>>> options to improve coverage.")
>>> +
>>>print(
>>>f"# Totals: pass:{ksft_cnt['pass']} fail:{ksft_cnt['fail']}
>>> xfail:0 xpass:0 skip:{ksft_cnt['skip']} error:0"
>>>)
>>> diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh
>>> b/tools/testing/selftests/kselftest/ktap_helpers.sh
>>> index 79a125eb24c2..a4211221ccd6 100644
>>> --- a/tools/testing/selftests/kselftest/ktap_helpers.sh
>>> +++ b/tools/testing/selftests/kselftest/ktap_helpers.sh
>>> @@ -107,5 +107,9 @@ ktap_finished() {
>>>}
>>> ktap_print_totals() {
>>> +if [ "$KTAP_CNT_SKIP" -gt 0 ]; then
>>> +echo "# Skipped tests detected. " \
>>> +"Consider enabling relevant config options to improve
>>> coverage."
>>> +fi
>>>echo "# Totals: pass:$KTAP_CNT_PASS fail:$KTAP_CNT_FAIL xfail:0
>>> xpass:0 skip:$KTAP_CNT_SKIP error:0"
>>>}
>>
>> thanks,
>> -- Shuah
>>
>
Thanks,
Laura
[PATCH v2] selftests: Warn about skipped tests in result summary
Update the functions that print the test totals at the end of a selftest
to include a warning message when skipped tests are detected. The
message advises users that skipped tests may indicate missing
configuration options and suggests enabling them to improve coverage.
Signed-off-by: Laura Nao
---
Changes in v2:
- Included count of skipped tests in the warning message
---
tools/testing/selftests/kselftest.h | 5 +
tools/testing/selftests/kselftest/ksft.py | 3 +++
tools/testing/selftests/kselftest/ktap_helpers.sh | 4
3 files changed, 12 insertions(+)
diff --git a/tools/testing/selftests/kselftest.h
b/tools/testing/selftests/kselftest.h
index 29fedf609611..249201664bcb 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -147,6 +147,11 @@ static inline void ksft_set_plan(unsigned int plan)
static inline void ksft_print_cnts(void)
{
+ if (ksft_cnt.ksft_xskip > 0)
+ printf(
+ "# %u skipped test(s) detected. Consider enabling
relevant config options to improve coverage.\n",
+ ksft_cnt.ksft_xskip
+ );
if (ksft_plan != ksft_test_num())
printf("# Planned tests != run tests (%u != %u)\n",
ksft_plan, ksft_test_num());
diff --git a/tools/testing/selftests/kselftest/ksft.py
b/tools/testing/selftests/kselftest/ksft.py
index bf215790a89d..0e030837fc17 100644
--- a/tools/testing/selftests/kselftest/ksft.py
+++ b/tools/testing/selftests/kselftest/ksft.py
@@ -27,6 +27,9 @@ def set_plan(num_tests):
def print_cnts():
+if ksft_cnt['skip'] > 0:
+print(f"# {ksft_cnt['skip']} skipped test(s) detected. Consider
enabling relevant config options to improve coverage.")
+
print(
f"# Totals: pass:{ksft_cnt['pass']} fail:{ksft_cnt['fail']} xfail:0
xpass:0 skip:{ksft_cnt['skip']} error:0"
)
diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh
b/tools/testing/selftests/kselftest/ktap_helpers.sh
index 79a125eb24c2..531094d81f03 100644
--- a/tools/testing/selftests/kselftest/ktap_helpers.sh
+++ b/tools/testing/selftests/kselftest/ktap_helpers.sh
@@ -107,5 +107,9 @@ ktap_finished() {
}
ktap_print_totals() {
+ if [ "$KTAP_CNT_SKIP" -gt 0 ]; then
+ echo "# $KTAP_CNT_SKIP skipped test(s) detected. " \
+ "Consider enabling relevant config options to improve
coverage."
+ fi
echo "# Totals: pass:$KTAP_CNT_PASS fail:$KTAP_CNT_FAIL xfail:0 xpass:0
skip:$KTAP_CNT_SKIP error:0"
}
--
2.30.2
[PATCH] selftests: Warn about skipped tests in result summary
Update the functions that print the test totals at the end of a selftest
to include a warning message when skipped tests are detected. The
message advises users that skipped tests may indicate missing
configuration options and suggests enabling them to improve coverage.
Signed-off-by: Laura Nao
---
This patch follows up on a previous discussion[1] and aims to highlight
skipped tests for the user's attention.
[1]
https://lore.kernel.org/lkml/2bb2d338-cd00-4ac2-b8bd-5579eae82...@linuxfoundation.org/
---
tools/testing/selftests/kselftest.h | 4
tools/testing/selftests/kselftest/ksft.py | 3 +++
tools/testing/selftests/kselftest/ktap_helpers.sh | 4
3 files changed, 11 insertions(+)
diff --git a/tools/testing/selftests/kselftest.h
b/tools/testing/selftests/kselftest.h
index 29fedf609611..d3f64b333acd 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -147,6 +147,10 @@ static inline void ksft_set_plan(unsigned int plan)
static inline void ksft_print_cnts(void)
{
+ if (ksft_cnt.ksft_xskip > 0)
+ printf(
+ "# Skipped tests detected. Consider enabling relevant
config options to improve coverage.\n"
+ );
if (ksft_plan != ksft_test_num())
printf("# Planned tests != run tests (%u != %u)\n",
ksft_plan, ksft_test_num());
diff --git a/tools/testing/selftests/kselftest/ksft.py
b/tools/testing/selftests/kselftest/ksft.py
index bf215790a89d..7675a15a1264 100644
--- a/tools/testing/selftests/kselftest/ksft.py
+++ b/tools/testing/selftests/kselftest/ksft.py
@@ -27,6 +27,9 @@ def set_plan(num_tests):
def print_cnts():
+if ksft_cnt['skip'] > 0:
+print("# Skipped tests detected. Consider enabling relevant config
options to improve coverage.")
+
print(
f"# Totals: pass:{ksft_cnt['pass']} fail:{ksft_cnt['fail']} xfail:0
xpass:0 skip:{ksft_cnt['skip']} error:0"
)
diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh
b/tools/testing/selftests/kselftest/ktap_helpers.sh
index 79a125eb24c2..a4211221ccd6 100644
--- a/tools/testing/selftests/kselftest/ktap_helpers.sh
+++ b/tools/testing/selftests/kselftest/ktap_helpers.sh
@@ -107,5 +107,9 @@ ktap_finished() {
}
ktap_print_totals() {
+ if [ "$KTAP_CNT_SKIP" -gt 0 ]; then
+ echo "# Skipped tests detected. " \
+ "Consider enabling relevant config options to improve
coverage."
+ fi
echo "# Totals: pass:$KTAP_CNT_PASS fail:$KTAP_CNT_FAIL xfail:0 xpass:0
skip:$KTAP_CNT_SKIP error:0"
}
--
2.30.2
