From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v4 1/2] lib: introduce tst_timeout_remaining()
Date: Thu, 30 Aug 2018 14:02:56 +0200 [thread overview]
Message-ID: <20180830120256.GD6363@rei.lan> (raw)
In-Reply-To: <1443598375.43662526.1535630056840.JavaMail.zimbra@redhat.com>
Hi!
> > > Fair enough, also the alarm() in the test library pid is set before we
> > > run the test setup, so if the test setup would take a few seconds we
> > > will be off with the calculation. Although that could be fixed by
> > > calling heartbeat before we run the loop in testrun(), which I guess
> > > should be done anyway. That in turn would allow for your patch to have
> > > the clock_gettime only in the heartbeat function, right?
>
> Correct. We could replace it with call to hearbeat():
>
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -929,9 +929,7 @@ static void testrun(void)
> unsigned long long stop_time = 0;
> int cont = 1;
>
> - if (tst_clock_gettime(CLOCK_MONOTONIC, &tst_start_time))
> - tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> -
> + heartbeat();
> add_paths();
> do_test_setup();
>
> but then we should get rid of extra alarm() call in tst_set_timeout(),
> because new hearbeat() call will do it anyway (it will send signal,
> and handler in lib will call alarm()):
>
> @@ -1038,9 +1036,7 @@ void tst_set_timeout(int timeout)
> results->timeout/3600, (results->timeout%3600)/60,
> results->timeout % 60);
>
> - if (getpid() == lib_pid)
> - alarm(results->timeout);
> - else
> + if (getpid() != lib_pid)
> heartbeat();
> }
That would mean that the test library will not timeout unless the child
process managed to send it a signal, so I would like to keep it there,
since it's more robust that way.
Well, we may change this, so that the first alarm in the test library
runs with something as 30 seconds, which should be enough before we get
the heartbeat() from the child that would reset the alarm with a correct
timemout.
> >
> > Actually we would have to do the heartbeat before and after the setup.
>
> Why after setup? Doesn't the time spent in setup count towards test time?
Actually it does, the setup + first iteration of the test share the
timeout, since the first call to alarm(timeout) in the test library
happens before we call the test setup. Subsequent iterations does not
run the setup at all, so the whole timeout applies only to the actuall
test function.
This haven't been a problem since our tests are usually very fast, but
it would probably be better if we do heartbeat() before and after
do_test_setup().
--
Cyril Hrubis
chrubis@suse.cz
next prev parent reply other threads:[~2018-08-30 12:02 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-30 8:55 [LTP] [PATCH v4 1/2] lib: introduce tst_timeout_remaining() Jan Stancek
2018-08-30 8:55 ` [LTP] [PATCH v4 2/2] move_pages12: end early if runtime gets close to test time Jan Stancek
2018-08-30 10:42 ` [LTP] [PATCH v4 1/2] lib: introduce tst_timeout_remaining() Cyril Hrubis
2018-08-30 11:01 ` Jan Stancek
2018-08-30 11:22 ` Cyril Hrubis
2018-08-30 11:30 ` Cyril Hrubis
2018-08-30 11:54 ` Jan Stancek
2018-08-30 12:02 ` Cyril Hrubis [this message]
2018-08-30 12:17 ` Jan Stancek
2018-08-30 12:41 ` Cyril Hrubis
2018-09-03 5:50 ` Jan Stancek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180830120256.GD6363@rei.lan \
--to=chrubis@suse.cz \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox