DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: David Marchand <david.marchand@redhat.com>,
	Harry van Haaren <harry.van.haaren@intel.com>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: dev <dev@dpdk.org>, Aaron Conole <aconole@redhat.com>,
	dpdk stable <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2] eal/service: fix exit by resetting service lcores
Date: Mon, 6 Apr 2020 11:30:46 +0100	[thread overview]
Message-ID: <c6819060-183a-108f-258c-832f1eb120d6@intel.com> (raw)
In-Reply-To: <CAJFAV8zi7BBLvbDkrNjZJnewK0fbowjqxqxBPR3wDYShN+2j9g@mail.gmail.com>

On 13-Mar-20 10:04 AM, David Marchand wrote:
> On Wed, Mar 11, 2020 at 3:39 PM Harry van Haaren
> <harry.van.haaren@intel.com> wrote:
>>
>> This commit releases all service cores from their role,
>> returning them to ROLE_RTE on rte_service_finalize().
>>
>> This may fix an issue relating to the service cores causing
> 
> s/may fix/fixes/
> 
>> a race-condition on eal_cleanup(), where the service core
>> could still be executing while the main thread has already
>> free-d the service memory, leading to a segfault.
>>
>> Fixes: 21698354c832 ("service: introduce service cores concept")
> 
> Replaced with:
> Fixes: da23f0aa87d8 ("service: fix memory leak with new function")
> 
>> Cc: stable@dpdk.org
>>
>> Reported-by: David Marchand <david.marchand@redhat.com>
>> Reported-by: Aaron Conole <aconole@redhat.com>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>> Acked-by: Aaron Conole <aconole@redhat.com>
> 
> Applied, thanks.
> 
> 

This patch breaks a couple of apps (or rather the apps were broken to 
begin with, but the brokenness has been exposed with this patch).

A "good" way to handle a SIGINT is to catch it, set some kind of global 
exit flag, and exit the signal handler, so that all of the threads see 
the exit flag, stop spinning, and exit the main loop and proceed to 
gracefully shutdown. That's what majority of our apps do.

A bad way to handle SIGINT is to call rte_exit() inside the signal 
handler, without setting any global exit flags. Since rte_exit() now 
waits for all of the threads to stop, the exit will never actually 
happen because threads can't stop without an exit signal, and no exit 
signal was provided by the signal handler.

Affected apps:

* l3fwd-power (i'm preparing a patch)
* ip_reassembly (see main.c:988) - +Konstantin

There are also a bunch of apps that simply call exit(0) and do unclean 
shutdown without DPDK cleanup, and also apps i have no idea what they're 
doing (call kill() on themselves in the SIGINT handler? l3fwd-cat does 
that, so do a bunch of others), but this is probably a bigger problem 
that should be addressed separately.

-- 
Thanks,
Anatoly

  reply	other threads:[~2020-04-06 10:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-10 13:33 [dpdk-dev] [PATCH] " Harry van Haaren
2020-03-10 16:31 ` David Marchand
2020-03-10 16:38   ` Van Haaren, Harry
2020-03-10 17:44     ` Aaron Conole
2020-03-10 19:14       ` Aaron Conole
2020-03-11  9:09     ` David Marchand
2020-03-11 14:39 ` [dpdk-dev] [PATCH v2] " Harry van Haaren
2020-03-11 16:15   ` David Marchand
2020-03-11 16:21     ` Van Haaren, Harry
2020-03-12  8:59       ` David Marchand
2020-03-11 17:08     ` Aaron Conole
2020-03-12  9:03       ` David Marchand
2020-03-13 10:04   ` David Marchand
2020-04-06 10:30     ` Burakov, Anatoly [this message]
2020-04-14 13:22       ` Aaron Conole

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=c6819060-183a-108f-258c-832f1eb120d6@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=aconole@redhat.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=harry.van.haaren@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=stable@dpdk.org \
    /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).