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 5F4C8A09E0; Fri, 11 Dec 2020 16:43:04 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 92017AC9C; Fri, 11 Dec 2020 16:43:02 +0100 (CET) Received: from smtp1.iitb.ac.in (smtpd9.iitb.ac.in [103.21.126.64]) by dpdk.org (Postfix) with ESMTP id 404DEAC9A for ; Fri, 11 Dec 2020 14:39:02 +0100 (CET) Received: from ldns1.iitb.ac.in (ldns1.iitb.ac.in [10.200.12.1]) by smtp1.iitb.ac.in (Postfix) with SMTP id 55FE7104D39C for ; Fri, 11 Dec 2020 19:09:00 +0530 (IST) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.iitb.ac.in 55FE7104D39C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iitb.ac.in; s=mail; t=1607693940; bh=Jc/eLT5Uj58BGi7tIRjgIDnfSyZiD0vMQAtwmFzR9xA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=k5XgwLmCiXXXUF/h4d0LN5A/wFwg6H7wZRamlCFyjYp3TsSCMTkXlQsCsYOoDcchG yVf4EBAoFgZoKQx08FZDC+e6YUZt9T30rju5pnHG1Fn9BK5w33jf6pp2YZ3g8qxTpP lMX91VPqUSdMNliZb6+PgITkfI/mcXFxXsaZRAY8= Received: (qmail 30748 invoked by uid 510); 11 Dec 2020 19:09:00 +0530 X-Qmail-Scanner-Diagnostics: from 10.200.1.25 by ldns1 (envelope-from , uid 501) with qmail-scanner-2.11 spamassassin: 3.4.1. mhr: 1.0. {clamdscan: 0.101.4/26014} Clear:RC:1(10.200.1.25):SA:0(0.0/7.0):. Processed in 2.598753 secs; 11 Dec 2020 19:09:00 +0530 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on ldns1.iitb.ac.in X-Spam-Level: X-Spam-Status: No, score=0.0 required=7.0 tests=IITB_ORIG,PROPER_IITB_MSGID, T_RP_MATCHES_RCVD,URIBL_BLOCKED autolearn=disabled version=3.4.1 X-Spam-Pyzor: Reported 0 times. X-Envelope-From: prateekag@cse.iitb.ac.in X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received: from unknown (HELO ldns1.iitb.ac.in) (10.200.1.25) by ldns1.iitb.ac.in with SMTP; 11 Dec 2020 19:08:57 +0530 Received: from mail.cse.iitb.ac.in (miller.cse.iitb.ac.in [10.129.3.1]) by ldns1.iitb.ac.in (Postfix) with ESMTP id 5B305360048 for ; Fri, 11 Dec 2020 19:08:57 +0530 (IST) Received: by mail.cse.iitb.ac.in (Postfix, from userid 5001) id 56E5EF40D1E; Fri, 11 Dec 2020 19:08:57 +0530 (IST) Received: from www.cse.iitb.ac.in (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by mail.cse.iitb.ac.in (Postfix) with ESMTPSA id CB5DBF40D1C; Fri, 11 Dec 2020 19:08:56 +0530 (IST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 11 Dec 2020 19:08:56 +0530 From: prateekag To: "Ananyev, Konstantin" Cc: "Li, Xiaoyun" , Prateek Agarwal , dev@dpdk.org, thomas@monjalon.net In-Reply-To: References: <20201204175151.11868-1-pratekag@gmail.com> <92a8857e311142921587ec49fc41b2ab@cse.iitb.ac.in> Message-ID: X-Sender: prateekag@cse.iitb.ac.in User-Agent: Roundcube Webmail/1.3.9 X-Mailman-Approved-At: Fri, 11 Dec 2020 16:43:01 +0100 Subject: Re: [dpdk-dev] [PATCH] Remove printf from signal handler. 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" Hi, We reused some code snippets of signal handlers in one of our projects and faced this problem of deadlock when the user sent an interrupt signal (ctrl-C) in quick succession. This is a non-deterministic bug and highly unlikely to be observed frequently. The main idea behind pointing out was to look at the possible solutions suggested by the community. Another reason was to make everyone aware of this bug. Anyways, leaving a comment in the source or mentioning this in documentation may not be a bad idea. Regards Prateek Agarwal On 2020-12-10 14:54, Ananyev, Konstantin wrote: >> Hi >> I don't have a good solution here. >> Removing doesn't seem a good idea because there are many important alerts >> or reminders in all of those function calls and they may be >> called in other places too. >> I think replacing with "write" can't solve the problem too. Because >> behavior like stopping port will call driver functions. It's not very >> reasonable to me that replacing all possible logs. >> Use a flag and check the flag for each possible thread? Most examples >> already do this way. But for complicated app, it will be tricky and >> may cause other issues. > > I wonder do we really have to try to 'fix' that? > I don't remember anyone complained about such problem in 10 years DPDK > exists. > Konstantin > >> >> Maybe other people have better ideas? >> >> BRs >> Xiaoyun >> >> > -----Original Message----- >> > From: prateekag >> > Sent: Thursday, December 10, 2020 08:28 >> > To: Li, Xiaoyun >> > Cc: Prateek Agarwal ; dev@dpdk.org; >> > thomas@monjalon.net >> > Subject: Re: [dpdk-dev] [PATCH] Remove printf from signal handler. >> > >> > Hi >> > Agree. What is the way out? The printfs you mentioned look like important >> > messages and may have been called outside signal handler routines. >> > Shall they be removed as well or converted to "write" function call? Or we live >> > with the possiblity of deadlock howsoever unlikely. >> > Regards >> > Prateek Agarwal >> > >> > >> > On 2020-12-08 08:28, Li, Xiaoyun wrote: >> > > Hi >> > > I don't object with all the removing of printf. >> > > Just one concern, I don't think you actually solved the problem in >> > > this patch. >> > > >> > > Take testpmd as an example, the signal_handler includes many >> > > complicated actions after that very first printf like force_quit() >> > > which includes stop port, close port, hotplug... and of course a lot of printf in it. >> > > So only removing the first printf doesn't actually solve the issue you >> > > mentioned. >> > > >> > > And many examples do similar things as testpmd, they have the same >> > > issues too. >> > > >> > > BRs >> > > Xiaoyun >> > > >> > >> -----Original Message----- >> > >> From: dev On Behalf Of Prateek Agarwal >> > >> Sent: Saturday, December 5, 2020 01:52 >> > >> To: dev@dpdk.org >> > >> Cc: thomas@monjalon.net; Prateek Agarwal ; >> > >> Prateek Agarwal >> > >> Subject: [dpdk-dev] [PATCH] Remove printf from signal handler. >> > >> >> > >> printf is not async-signal safe. Using printf in signal handlers may >> > >> lead to deadlock. Removed printf from signal handlers present in >> > >> several applications. >> > >> >> > >> Signed-off-by: Prateek Agarwal >> > >> --- >> > >> app/pdump/main.c | 2 -- >> > >> app/test-eventdev/evt_main.c | 4 ---- >> > >> app/test-flow-perf/main.c | 3 --- >> > >> app/test-pmd/testpmd.c | 2 -- >> > >> app/test/test_pmd_perf.c | 1 - >> > >> 5 files changed, 12 deletions(-) >> > >> >> > >> diff --git a/app/pdump/main.c b/app/pdump/main.c index >> > >> b34bf3353..380f0ea0f 100644 >> > >> --- a/app/pdump/main.c >> > >> +++ b/app/pdump/main.c >> > >> @@ -573,8 +573,6 @@ static void >> > >> signal_handler(int sig_num) >> > >> { >> > >> if (sig_num == SIGINT) { >> > >> - printf("\n\nSignal %d received, preparing to exit...\n", >> > >> - sig_num); >> > >> quit_signal = 1; >> > >> } >> > >> } >> > >> diff --git a/app/test-eventdev/evt_main.c >> > >> b/app/test-eventdev/evt_main.c index a8d304bab..51d5897f8 100644 >> > >> --- a/app/test-eventdev/evt_main.c >> > >> +++ b/app/test-eventdev/evt_main.c >> > >> @@ -22,12 +22,8 @@ signal_handler(int signum) { >> > >> int i; >> > >> static uint8_t once; >> > >> - >> > >> if ((signum == SIGINT || signum == SIGTERM) && !once) { >> > >> once = true; >> > >> - printf("\nSignal %d received, preparing to exit...\n", >> > >> - signum); >> > >> - >> > >> if (test != NULL) { >> > >> /* request all lcores to exit from the main loop */ >> > >> *(int *)test->test_priv = true; >> > >> diff --git a/app/test-flow-perf/main.c b/app/test-flow-perf/main.c >> > >> index >> > >> 03d01a8b5..aeb0ef3b0 100644 >> > >> --- a/app/test-flow-perf/main.c >> > >> +++ b/app/test-flow-perf/main.c >> > >> @@ -1001,9 +1001,6 @@ static void >> > >> signal_handler(int signum) >> > >> { >> > >> if (signum == SIGINT || signum == SIGTERM) { >> > >> - printf("\n\nSignal %d received, preparing to exit...\n", >> > >> - signum); >> > >> - printf("Error: Stats are wrong due to sudden signal!\n\n"); >> > >> force_quit = true; >> > >> } >> > >> } >> > >> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index >> > >> 33fc0fddf..7ec87e7fd 100644 >> > >> --- a/app/test-pmd/testpmd.c >> > >> +++ b/app/test-pmd/testpmd.c >> > >> @@ -3794,8 +3794,6 @@ static void >> > >> signal_handler(int signum) >> > >> { >> > >> if (signum == SIGINT || signum == SIGTERM) { >> > >> - printf("\nSignal %d received, preparing to exit...\n", >> > >> - signum); >> > >> #ifdef RTE_LIB_PDUMP >> > >> /* uninitialize packet capture framework */ >> > >> rte_pdump_uninit(); >> > >> diff --git a/app/test/test_pmd_perf.c b/app/test/test_pmd_perf.c >> > >> index >> > >> 4db816a36..58cb84401 100644 >> > >> --- a/app/test/test_pmd_perf.c >> > >> +++ b/app/test/test_pmd_perf.c >> > >> @@ -319,7 +319,6 @@ signal_handler(int signum) { >> > >> /* USR1 signal, stop testing */ >> > >> if (signum == SIGUSR1) { >> > >> - printf("Force Stop!\n"); >> > >> stop = 1; >> > >> } >> > >> >> > >> -- >> > >> 2.25.1