diff mbox

[dpdk-dev,v7,08/11] eal: replace rte_panic instances in interrupts thread

Message ID 1524608213-2080-9-git-send-email-arnon@qwilt.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Arnon Warshavsky April 24, 2018, 10:16 p.m. UTC 
  replace panic calls with log and return value.
Thread function removes the noreturn attribute.

Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

Comments

Anatoly Burakov April 25, 2018, 9:14 a.m. UTC | #1 
On 24-Apr-18 11:16 PM, Arnon Warshavsky wrote:
> replace panic calls with log and return value.
> Thread function removes the noreturn attribute.
> 
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---

Just a general comment - i'm not too familiar with this code, but it 
looks like all of these failures will only happen on thread init. Can we 
make sure it starts?

You can use similar approach to Olivier's (recently merged) thread 
affinity patches, with pthread barriers etc. to ensure the thread has 
initialized properly, pthread_cancel() it if it didn't, and return -1 on 
thread init.
Arnon Warshavsky April 25, 2018, 9:37 a.m. UTC | #2 
> Just a general comment - i'm not too familiar with this code, but it looks
> like all of these failures will only happen on thread init. Can we make
> sure it starts?
>
> You can use similar approach to Olivier's (recently merged) thread
> affinity patches, with pthread barriers etc. to ensure the thread has
> initialized properly, pthread_cancel() it if it didn't, and return -1 on
> thread init.


Thanks for catching this one. I am now reverting it as well from this set,
to be properly handled in a set dedicated to the threading issue
diff mbox

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 58e9328..77e6f2a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -785,7 +785,7 @@  struct rte_intr_source {
  * @return
  *  never return;
  */
-static __attribute__((noreturn)) void *
+static void *
 eal_intr_thread_main(__rte_unused void *arg)
 {
 	struct epoll_event ev;
@@ -803,8 +803,11 @@  static __attribute__((noreturn)) void *
 
 		/* create epoll fd */
 		int pfd = epoll_create(1);
-		if (pfd < 0)
-			rte_panic("Cannot create epoll instance\n");
+		if (pfd < 0) {
+			RTE_LOG(CRIT, EAL, "%s(): Cannot create epoll instance\n",
+					__func__);
+			return NULL;
+		}
 
 		pipe_event.data.fd = intr_pipe.readfd;
 		/**
@@ -813,8 +816,11 @@  static __attribute__((noreturn)) void *
 		 */
 		if (epoll_ctl(pfd, EPOLL_CTL_ADD, intr_pipe.readfd,
 						&pipe_event) < 0) {
-			rte_panic("Error adding fd to %d epoll_ctl, %s\n",
+			RTE_LOG(CRIT, EAL, "%s(): Error adding fd to %d "
+					"epoll_ctl, %s\n",
+					__func__,
 					intr_pipe.readfd, strerror(errno));
+			return NULL;
 		}
 		numfds++;
 
@@ -831,9 +837,12 @@  static __attribute__((noreturn)) void *
 			 * into wait list.
 			 */
 			if (epoll_ctl(pfd, EPOLL_CTL_ADD,
-					src->intr_handle.fd, &ev) < 0){
-				rte_panic("Error adding fd %d epoll_ctl, %s\n",
-					src->intr_handle.fd, strerror(errno));
+					src->intr_handle.fd, &ev) < 0) {
+				RTE_LOG(CRIT, EAL, "%s(): Error adding fd %d epoll_ctl, %s\n",
+					__func__,
+					src->intr_handle.fd,
+					strerror(errno));
+				return NULL;
 			}
 			else
 				numfds++;
@@ -848,6 +857,8 @@  static __attribute__((noreturn)) void *
 		 */
 		close(pfd);
 	}
+
+	return NULL;
 }
 
 int