From: Thomas Monjalon <thomas@monjalon.net>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"pbhagavatula@caviumnetworks.com"
<pbhagavatula@caviumnetworks.com>
Subject: Re: [PATCH v2] test: fix debug autotest with eal cleanup addition
Date: Wed, 31 Jan 2018 15:31:41 +0100 [thread overview]
Message-ID: <10693804.qgnIZzW71k@xps> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB977258905663C3@IRSMSX103.ger.corp.intel.com>
31/01/2018 14:53, Ananyev, Konstantin:
> Hi Harry,
>
> From: Harry van Haaren
> >
> > Before this patch, the debug_autotest would call fork(),
> > call rte_panic() or rte_exit() in the child process, and
> > examine the return code to verify that rte_panic() and
> > rte_exit() were correctly reporting failures.
> >
> > With the inclusion of the rte_eal_cleanup() patch, rte_exit()
> > was modified to cleanly tear-down EAL allocations. Currently
> > only one library (service cores) is allocated by EAL at startup
> > and should be cleaned up. This library has a check on a normal
> > (non-hugepage) variable to protect against double cleanup. The
> > service cores finalize() function itself frees back hugepage mem.
> >
> > Given the fork() approach from the unit test, and the fact that
> > the double-free check is on an ordinary variable, causes multiple
> > child processed (fork()-ed from the unit-test runner) to attempt
> > to free the huge-page memory multiple times. The variable to
> > protect against double-cleanup was not effective, as the fork()
> > would restore it to show initialized in the next child.
> >
> > The solution is to call rte_service_finalize() *before* calling
> > fork(), which results in the service cores double-cleanup variable
> > to be zero before the fork(), and hence the child processes never
> > free the hugepage service-cores memory (correct behavior, as the
> > unit-test suite is still running, and owns the hugepages).
>
> Ok, you fixed it in UT, but what to do other apps that use fork()?
> Let say our examples/multi_process/l2fwd_fork uses fork() to
> spawn child processes instead of threads.
> Might be some generic way is needed: let say at fork time setup some
> global to indicate that it is a child process and it shouldn't call rte_finalize() or so.
> Konstantin
At first, we should discuss whether it is a good idea to support fork,
given that we have the "secondary process solution".
Then, if an improvement is needed, it should go in 18.05.
I think the fix in UT is good enough for 18.02.
next prev parent reply other threads:[~2018-01-31 14:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-30 18:21 [PATCH] test: fix debug autotest with eal cleanup addition Harry van Haaren
2018-01-30 18:26 ` [PATCH v2] " Harry van Haaren
2018-01-30 23:53 ` Thomas Monjalon
2018-01-31 13:53 ` Ananyev, Konstantin
2018-01-31 14:31 ` Thomas Monjalon [this message]
2018-01-31 14:54 ` Van Haaren, Harry
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=10693804.qgnIZzW71k@xps \
--to=thomas@monjalon.net \
--cc=dev@dpdk.org \
--cc=harry.van.haaren@intel.com \
--cc=konstantin.ananyev@intel.com \
--cc=pbhagavatula@caviumnetworks.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.