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 <dev-boun...@dpdk.org> On Behalf Of Prateek Agarwal
Sent: Saturday, December 5, 2020 01:52
To: dev@dpdk.org
Cc: tho...@monjalon.net; Prateek Agarwal <prate...@gmail.com>; Prateek
Agarwal <pratee...@cse.iitb.ac.in>
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 <pratee...@cse.iitb.ac.in>
---
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
