From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9CDA2A057B; Tue, 14 Apr 2020 15:22:40 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 745081C222; Tue, 14 Apr 2020 15:22:39 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by dpdk.org (Postfix) with ESMTP id 739AF1C1F6 for ; Tue, 14 Apr 2020 15:22:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1586870557; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6uO1HjIPFG/D1lbJeWdD5mtFG4FWH0Ou5HmB+yKkFHY=; b=SRz6ir4+RoxaaqbOfib8gJi80FkhYz2LeuyfpkhMGDF8cuiYhFjVfPHubZOYYUQg6TZzni 9gSBOAkztqnlrJaThLt9hLV38iPFhp9IzLRRHp5kjzjxqF6Gz9kjWrwcXNlHGL72jaEh4F 0z8+dbaw1YSpzUKqsbn42R074K26WII= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-508-fBPio5tcOdOWYms3P3ylpQ-1; Tue, 14 Apr 2020 09:22:36 -0400 X-MC-Unique: fBPio5tcOdOWYms3P3ylpQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 54D168017F5; Tue, 14 Apr 2020 13:22:35 +0000 (UTC) Received: from dhcp-25.97.bos.redhat.com (ovpn-116-136.phx2.redhat.com [10.3.116.136]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D435C5C1D6; Tue, 14 Apr 2020 13:22:28 +0000 (UTC) From: Aaron Conole To: "Burakov\, Anatoly" Cc: David Marchand , Harry van Haaren , "Ananyev\, Konstantin" , dev , dpdk stable References: <20200310133304.39951-1-harry.van.haaren@intel.com> <20200311143927.76021-1-harry.van.haaren@intel.com> Date: Tue, 14 Apr 2020 09:22:26 -0400 In-Reply-To: (Anatoly Burakov's message of "Mon, 6 Apr 2020 11:30:46 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v2] eal/service: fix exit by resetting service lcores X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" "Burakov, Anatoly" writes: > On 13-Mar-20 10:04 AM, David Marchand wrote: >> On Wed, Mar 11, 2020 at 3:39 PM Harry van Haaren >> 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 >>> Reported-by: Aaron Conole >>> Signed-off-by: David Marchand >>> Signed-off-by: Harry van Haaren >>> Acked-by: Aaron Conole >> >> 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. Yes, I don't consider it 'breaking' anything - exit in signal handlers is always a bad idea. I guess we should correct the examples to show this. > 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. I think one way to mitigate this is to register an at_exit() function that will check if eal is currently initialized and do the needed cleanup call. I don't know if there are any side-effects that we need to consider for it, though.