On 28/10/2024 11:29, Shuah Khan wrote: > On 10/27/24 18:50, Zhijian Li (Fujitsu) wrote: >> >> >> On 27/10/2024 08:28, Shuah Khan wrote: >>> On 10/24/24 19:39, Li Zhijian wrote: >>>> Currently, watchdog-test keep running until it gets a SIGINT. However, >>>> when watchdog-test is executed from the kselftests framework, where it >>>> launches test via timeout which will send SIGTERM in time up. This could >>>> lead to >>>> 1. watchdog haven't stop, a watchdog reset is triggered to reboot the OS >>>> in silent. >>>> 2. kselftests gets an timeout exit code, and judge watchdog-test as >>>> 'not ok' >>>> >>> This test isn't really supposed to be run from kselftest framework. >>> This is the reason why it isn't included in the default run. >> >> May I know what's the default run, is it different from `make run_tests` ? > > No it isn't. "make kselftest" runs only the targets mentioned in the > selftests Makefile. That is considered the kselftest default run. Hey, Shuah, Thanks for your explanation. If that is the case, I do not have an urgent need for the current patch, expect I'd like to avoid the reboot issue after an accidentally `make run_tests` Some changes are make as below, please take a look. I will send it out we reach a consensus. commit 2296f9d88fde4921758a45bf160a7f1b9d4678a0 (HEAD) Author: Li Zhijian <lizhijian@xxxxxxxxxxx> Date: Mon Oct 28 11:54:03 2024 +0800 selftests/watchdog-test: Fix system accidentally reset after watchdog-test After `make run_tests` to run watchdog-test, a system reboot would happen due to watchdog not stop. ``` [ 1367.185172] watchdog: watchdog0: watchdog did not stop! ``` Fix it by registering a timeout signal in watchdog-test, where its signal handler will stop the watchdog. After that # timeout 1 ./watchdog-test Watchdog Ticking Away! . Stopping watchdog ticks... Signed-off-by: Li Zhijian <lizhijian@xxxxxxxxxxx> diff --git a/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c index bc71cbca0dde..97acb90f8b30 100644 --- a/tools/testing/selftests/watchdog/watchdog-test.c +++ b/tools/testing/selftests/watchdog/watchdog-test.c @@ -335,6 +335,10 @@ int main(int argc, char *argv[]) printf("Watchdog Ticking Away!\n"); signal(SIGINT, term); + /* + * Register the timeout signal + */ + signal(SIGTERM, term); while (1) { keep_alive(); > > There is a reason why watchdog isn't included in the default run. > It isn't intended to be run by users by default as this is test is > just for testing watchdog api > >> >> >>> >>>> This patch is prepare to fix above 2 issues >>> >>> This series needs a separate cover letter explaining how this problem is >>> being fixed. >> >> Cover letter is in this patch, see below: >> In addition, we can get the 'How' by reading the simple change in each change. > > That isn't enough to understand why this change is needed. > Send patch series with a cover letter explaining what you are > doing. > >> >> >>> >>>> >>>> Signed-off-by: Li Zhijian <lizhijian@xxxxxxxxxxx> >>>> --- >>>> Hey, >>>> Cover letter is here. >>>> >>>> It's notice that a OS reboot was triggerred after ran the watchdog-test >>>> in kselftests framwork 'make run_tests', that's because watchdog-test >>>> didn't stop feeding the watchdog after enable it. >>>> >>>> In addition, current watchdog-test didn't adapt to the kselftests >>>> framework which launchs the test with /usr/bin/timeout and no timeout >>>> is expected. >>>> --- > > thanks, > -- Shuah