[dpdk-dev,15/25] eal: do not panic on alarm init

Message ID 1485529023-5486-16-git-send-email-aconole@redhat.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel compilation fail Compilation issues

Commit Message

Aaron Conole Jan. 27, 2017, 2:56 p.m. UTC
  rte_eal_alarm_init() call uses the linux timerfd framework to create a
poll()-able timer using standard posix file operations.  This could fail
for a few reasons given in the man-pages, but many could be
corrected by the user application.  No need to panic.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
  

Comments

Stephen Hemminger Jan. 27, 2017, 4:31 p.m. UTC | #1
On Fri, 27 Jan 2017 09:56:53 -0500
Aaron Conole <aconole@redhat.com> wrote:

> +	if (rte_eal_alarm_init() < 0) {
> +		RTE_LOG (ERR, EAL, "Cannot init interrupt-handling thread\n");
> +		/* rte_eal_alarm_init sets rte_errno on failure. */
> +		errno = rte_errno;

Hmm. DPDK in general does not reset errno but instead uses error code
directly on return (best) or in some cases rte_errno
  
Bruce Richardson Jan. 27, 2017, 4:42 p.m. UTC | #2
On Fri, Jan 27, 2017 at 08:31:55AM -0800, Stephen Hemminger wrote:
> On Fri, 27 Jan 2017 09:56:53 -0500
> Aaron Conole <aconole@redhat.com> wrote:
> 
> > +	if (rte_eal_alarm_init() < 0) {
> > +		RTE_LOG (ERR, EAL, "Cannot init interrupt-handling thread\n");
> > +		/* rte_eal_alarm_init sets rte_errno on failure. */
> > +		errno = rte_errno;
> 
> Hmm. DPDK in general does not reset errno but instead uses error code
> directly on return (best) or in some cases rte_errno

I think we'll disagree on what way of returning error codes is best :-), but
yes, DPDK does not generally modify errno.
  
Aaron Conole Jan. 30, 2017, 4:52 p.m. UTC | #3
Bruce Richardson <bruce.richardson@intel.com> writes:

> On Fri, Jan 27, 2017 at 08:31:55AM -0800, Stephen Hemminger wrote:
>> On Fri, 27 Jan 2017 09:56:53 -0500
>> Aaron Conole <aconole@redhat.com> wrote:
>> 
>> > +	if (rte_eal_alarm_init() < 0) {
>> > +		RTE_LOG (ERR, EAL, "Cannot init interrupt-handling thread\n");
>> > +		/* rte_eal_alarm_init sets rte_errno on failure. */
>> > +		errno = rte_errno;
>> 
>> Hmm. DPDK in general does not reset errno but instead uses error code
>> directly on return (best) or in some cases rte_errno
>
> I think we'll disagree on what way of returning error codes is best :-), but
> yes, DPDK does not generally modify errno.

Okay, I'll drop the errno set.  I think it's a mistake from the first
version of the series (as RFC).

Thanks for the reviews, Bruce and Stephen!
  

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index b16313c..122d9c2 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -61,6 +61,7 @@ 
 #include <rte_launch.h>
 #include <rte_eal.h>
 #include <rte_eal_memconfig.h>
+#include <rte_errno.h>
 #include <rte_per_lcore.h>
 #include <rte_lcore.h>
 #include <rte_log.h>
@@ -860,8 +861,12 @@  rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	if (rte_eal_alarm_init() < 0)
-		rte_panic("Cannot init interrupt-handling thread\n");
+	if (rte_eal_alarm_init() < 0) {
+		RTE_LOG (ERR, EAL, "Cannot init interrupt-handling thread\n");
+		/* rte_eal_alarm_init sets rte_errno on failure. */
+		errno = rte_errno;
+		return -1;
+	}
 
 	if (rte_eal_timer_init() < 0)
 		rte_panic("Cannot init HPET or TSC timers\n");