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@collabora.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