From: Jan Stancek <jstancek@redhat.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v2] lib/tst_test.c: Run test in child process
Date: Wed, 8 Jun 2016 08:59:01 -0400 (EDT) [thread overview]
Message-ID: <1836941003.4597191.1465390741285.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20160608123056.GA26573@rei.lan>
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Wednesday, 8 June, 2016 2:30:56 PM
> Subject: Re: [LTP] [PATCH v2] lib/tst_test.c: Run test in child process
>
> Hi!
> > > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > > index b8ec246..eef54e4 100644
> > > --- a/lib/tst_test.c
> > > +++ b/lib/tst_test.c
> > > @@ -220,17 +220,19 @@ void tst_vres_(const char *file, const int lineno,
> > > int
> > > ttype,
> > > void tst_vbrk_(const char *file, const int lineno, int ttype,
> > > const char *fmt, va_list va) __attribute__((noreturn));
> > >
> > > -static void do_cleanup(void);
> > > +static void do_test_cleanup(void)
> > > +{
> > > + if (tst_test->cleanup)
> > > + tst_test->cleanup();
> > > +}
> > >
> > > void tst_vbrk_(const char *file, const int lineno, int ttype,
> > > const char *fmt, va_list va)
> > > {
> > > print_result(file, lineno, ttype, fmt, va);
> > >
> > > - if (getpid() == main_pid) {
> > > - do_cleanup();
> > > - cleanup_ipc();
> > > - }
> > > + if (getpid() == main_pid)
> > > + do_test_cleanup();
> > >
> >
> > Not directly related to this patch, but I noticed that we don't
> > seem to cleanup_ipc if we hit TBROK outside of main test pid.
>
> The cleanup_ipc() is intended to be executed just before the main pid
> exits since it unlinks the shm file.
So, if we fail a SAFE macro somewhere in lib/tst_test.c,
I don't see we ever call cleanup_ipc().
tst_run_tcases
SAFE_WAITPID(test_pid, &status, 0);
tst_brkm
tst_brkm_
tst_brk_
tst_vbrk_
exit
> If child TBROKs it's catched in the
> check_child_status() in the parent, tst_brk() is called which will call
> the cleanup_ipc().
Not anymore. After this patch the only place that now calls
cleanup_ipc() is do_exit().
What I'm thinking is something like this on top of your patch:
diff --git a/lib/tst_test.c b/lib/tst_test.c
index eef54e49bafb..1a0a9494af26 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -40,7 +40,7 @@ struct tst_test *tst_test;
static char tmpdir_created;
static int iterations = 1;
static float duration = -1;
-static pid_t main_pid;
+static pid_t main_pid, lib_pid;
struct results {
int passed;
@@ -234,6 +234,9 @@ void tst_vbrk_(const char *file, const int lineno, int ttype,
if (getpid() == main_pid)
do_test_cleanup();
+ if (getpid() == lib_pid)
+ do_exit();
+
exit(TTYPE_RESULT(ttype));
}
@@ -673,6 +676,7 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
int status;
char *mul;
+ lib_pid = getpid();
tst_test = self;
TCID = tst_test->tid;
>
> I guess that we may close and unmap the shm even in the children but
> wouldn't that happen anyway as the process exits?
>
> > > +void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
> > > +{
> > > + int status;
> > > + char *mul;
> > > +
> > > + tst_test = self;
> > > + TCID = tst_test->tid;
> > > +
> > > + do_setup(argc, argv);
> > > +
> > > + if (tst_test->timeout)
> > > + timeout = tst_test->timeout;
> >
> > Can you think of a testcase where we would want to disable timeout?
>
> At the moment I do not remember a test that would need timeout to be
> turned off. And if we find that it's necessary we can always disable in
> case that tst_test->timeout < 0.
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
next prev parent reply other threads:[~2016-06-08 12:59 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-07 11:58 [LTP] [PATCH v2] lib/tst_test.c: Run test in child process Cyril Hrubis
2016-06-08 8:17 ` Jan Stancek
2016-06-08 12:30 ` Cyril Hrubis
2016-06-08 12:59 ` Jan Stancek [this message]
2016-06-08 13:15 ` Cyril Hrubis
2016-06-08 13:36 ` Jan Stancek
2016-06-08 13:55 ` Cyril Hrubis
2016-06-08 14:06 ` Cyril Hrubis
2016-06-08 14:31 ` Jan Stancek
2016-06-08 14:32 ` Cyril Hrubis
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=1836941003.4597191.1465390741285.JavaMail.zimbra@redhat.com \
--to=jstancek@redhat.com \
--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