[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6b2bca6d-61e8-40f6-b3b1-3a81dfb2b397@linuxfoundation.org>
Date: Wed, 3 Jul 2024 17:10:59 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Laura Nao <laura.nao@...labora.com>
Cc: kernel@...labora.com, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, shuah@...nel.org,
Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 1/2] selftests/watchdog: limit ping loop and allow
configuring the number of pings
On 7/3/24 08:48, Laura Nao wrote:
> On 6/27/24 20:48, Shuah Khan wrote:
>> On 5/6/24 05:13, Laura Nao wrote:
>>> In order to run the watchdog selftest with the kselftest runner, the
>>> loop responsible for pinging the watchdog should be finite. This
>>> change limits the loop to 5 iterations by default and introduces a new
>>> '-c' option to adjust the number of pings as needed.
>>
>> This patch makes the test run finite in all cases changing the bevavior
>> to run it forever?
>
> Correct.
>
>>>
>>> Signed-off-by: Laura Nao <laura.nao@...labora.com>
>>> ---
>>> tools/testing/selftests/watchdog/watchdog-test.c | 16 ++++++++++++++--
>>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/watchdog/watchdog-test.c
>>> b/tools/testing/selftests/watchdog/watchdog-test.c
>>> index bc71cbca0dde..786cc5a26206 100644
>>> --- a/tools/testing/selftests/watchdog/watchdog-test.c
>>> +++ b/tools/testing/selftests/watchdog/watchdog-test.c
>>> @@ -24,16 +24,18 @@
>>> #include <linux/watchdog.h>
>>> #define DEFAULT_PING_RATE 1
>>> +#define DEFAULT_PING_COUNT 5
>>> 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 +92,8 @@ 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\tSet number of pings to C (default
>>> %d)\n",
>>> + DEFAULT_PING_COUNT);
>>> 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 +176,7 @@ int main(int argc, char *argv[])
>>> {
>>> int flags;
>>> unsigned int ping_rate = DEFAULT_PING_RATE;
>>> + unsigned int ping_count = DEFAULT_PING_COUNT;
>>> int ret;
>>> int c;
>>> int oneshot = 0;
>>> @@ -248,6 +253,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)
>>> + ping_count = DEFAULT_PING_COUNT;
>>> + printf("Number of pings set to %u.\n", ping_count);
>>> + break;
>>> case 's':
>>> flags = 0;
>>> oneshot = 1;
>>> @@ -336,9 +347,10 @@ int main(int argc, char *argv[])
>>> signal(SIGINT, term);
>>> - while (1) {
>>> + while (ping_count > 0) {
>>> keep_alive();
>>> sleep(ping_rate);
>>> + ping_count--;
>>
>> So this test no longer runs forever?
>>
>
> That's correct, with this patch applied the test no longer runs forever.
> I understand you prefer the current behavior - how about keeping the
> keep_alive() loop infinite by default and only making it finite when the
> -c argument is passed? Would that be reasonable?
>
Yes. I am open to taking the patch if the default behavior doesn't change.
thanks,
-- Shuah
Powered by blists - more mailing lists