From: David Marchand <david.marchand@redhat.com>
To: "Zeng, ZhichaoX" <zhichaox.zeng@intel.com>,
Bruce Richardson <bruce.richardson@intel.com>,
Aaron Conole <aconole@redhat.com>,
Olivier Matz <olivier.matz@6wind.com>
Cc: dev <dev@dpdk.org>, Qiming Yang <qiming.yang@intel.com>,
Harman Kalra <hkalra@marvell.com>,
Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2] lib/eal: fix segfaults due to thread exit order
Date: Mon, 23 May 2022 15:00:12 +0200 [thread overview]
Message-ID: <CAJFAV8y0gZr81hiXyYOsU=T6rVP45HVyUktkpmWkO5f1oL5VzQ@mail.gmail.com> (raw)
In-Reply-To: <CAJFAV8xGoj-3gcg=FUTb91=gpE2cDst7MfuPxjpXa5-Nt=Hi2A@mail.gmail.com>
On Mon, May 23, 2022 at 2:10 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Mon, May 23, 2022 at 5:17 AM <zhichaox.zeng@intel.com> wrote:
> >
> > From: Zhichao Zeng <zhichaox.zeng@intel.com>
> >
> > The eal-intr-thread is not closed before memory cleanup in the
> > process of exiting. There is a small probability that when the
> > eal-intr-thread is about to use some pointers, the memory were
> > just cleaned, which cause the segment fault error caught by ASan.
> >
> > This patch close the eal-intr-thread before memory cleanup when
> > exiting to avoid segment fault.
>
> This breaks the debug_autotest unit test.
> It results in a segfault in a forked process executing
> rte_exit()/rte_eal_cleanup().
>
> That's probably because intr_thread thread does not exist in the forked process.
Reading fork() manual:
* The child process is created with a single thread—the one
that called fork(). The entire virtual address space of the parent is
replicated in the child, including the states of mutexes, condi‐
tion variables, and other pthreads objects; the use of
pthread_atfork(3) may be helpful for dealing with problems that this
can cause.
We may need a check like diff below.
But then, debug_autotest code seems dangerous, because it does exactly
what the added check wants to warn about.
Opinions?
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 1ef263434a..1e6fd01d5d 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -857,12 +857,25 @@ is_iommu_enabled(void)
return n > 2;
}
+static uint32_t run_once;
+
+static void warn_parent(void)
+{
+ RTE_LOG(WARNING, EAL, "fork() was called, DPDK won't work in the child "
+ "process unless it calls rte_eal_init()\n");
+}
+
+static void scratch_child(void)
+{
+ /* Scratch run_once so that a call to rte_eal_cleanup won't crash... */
+ __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
+}
+
/* Launch threads, called at application init(). */
int
rte_eal_init(int argc, char **argv)
{
int i, fctret, ret;
- static uint32_t run_once;
uint32_t has_run = 0;
const char *p;
static char logid[PATH_MAX];
@@ -1228,6 +1241,8 @@ rte_eal_init(int argc, char **argv)
eal_mcfg_complete();
+ pthread_atfork(NULL, warn_parent, scratch_child);
+
return fctret;
}
@@ -1257,6 +1272,9 @@ rte_eal_cleanup(void)
struct internal_config *internal_conf =
eal_get_internal_configuration();
+ if (__atomic_load_n(&run_once, __ATOMIC_RELAXED) == 0)
+ return 0;
+
if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
internal_conf->hugepage_file.unlink_existing)
rte_memseg_walk(mark_freeable, NULL);
--
David Marchand
next prev parent reply other threads:[~2022-05-23 13:00 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-17 16:04 [DPDK] eal/linux: " zhichaox.zeng
2022-05-17 8:29 ` Morten Brørup
2022-05-17 15:25 ` [EXT] " Harman Kalra
2022-05-18 14:39 ` [DPDK v2] lib/eal: " zhichaox.zeng
2022-05-23 11:16 ` [PATCH " zhichaox.zeng
2022-05-23 12:10 ` David Marchand
2022-05-23 13:00 ` David Marchand [this message]
2022-05-30 13:47 ` [PATCH v3] " zhichaox.zeng
2022-05-30 16:26 ` Stephen Hemminger
2022-05-30 16:28 ` Stephen Hemminger
2022-06-02 8:21 ` Zeng, ZhichaoX
2022-06-07 10:14 ` Zeng, ZhichaoX
2022-06-15 6:01 ` [PATCH v4] " zhichaox.zeng
2022-06-24 1:42 ` Zeng, ZhichaoX
2022-06-24 7:50 ` David Marchand
2022-06-30 10:38 ` Zeng, ZhichaoX
2022-06-30 12:24 ` Bruce Richardson
2022-09-06 2:51 ` [PATCH v5] lib/eal: fix segfaults in exiting Zhichao Zeng
2022-09-06 15:03 ` Stephen Hemminger
2022-09-07 8:53 ` Zeng, ZhichaoX
2022-10-11 5:25 ` [PATCH v6] " Zhichao Zeng
2022-10-11 14:04 ` Stephen Hemminger
2022-10-19 1:51 ` Zeng, ZhichaoX
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='CAJFAV8y0gZr81hiXyYOsU=T6rVP45HVyUktkpmWkO5f1oL5VzQ@mail.gmail.com' \
--to=david.marchand@redhat.com \
--cc=aconole@redhat.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=hkalra@marvell.com \
--cc=olivier.matz@6wind.com \
--cc=qiming.yang@intel.com \
--cc=thomas@monjalon.net \
--cc=zhichaox.zeng@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).