From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 884035A08 for ; Mon, 29 Dec 2014 17:18:32 +0100 (CET) Received: from [2001:470:8:a08:215:ff:fecc:4872] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1Y5d1W-0001SF-40; Mon, 29 Dec 2014 11:18:31 -0500 Date: Mon, 29 Dec 2014 11:18:24 -0500 From: Neil Horman To: Ravi Kerur Message-ID: <20141229161824.GC29258@localhost.localdomain> References: <1419694322-2114-1-git-send-email-rkerur@gmail.com> <1419694402-2215-1-git-send-email-rkerur@gmail.com> <20141228204253.GA23822@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 1/6] Move EAL common functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Dec 2014 16:18:33 -0000 On Mon, Dec 29, 2014 at 07:34:22AM -0800, Ravi Kerur wrote: > I will change the subject and make it more explicit. In addition, I was > thinking of adding function names which are moved as part of commit message > so that it is easier for everyone to check what is moved rather than going > through the patch. Do you think it would be helpful?? > yeah, adding that detail in the changelog, as well as the files updated in the subject would be a big help for historical purposes. Thanks! Neil > Thanks. > > On Sun, Dec 28, 2014 at 12:42 PM, Neil Horman wrote: > > > On Sat, Dec 27, 2014 at 10:33:17AM -0500, Ravi Kerur wrote: > > > Changes in v2 > > > 1. Remove rte_dump_registers() function since it is not implemented. > > > 2. Fix comment for _rte_exit() > > > > > > eal_debug.c has no difference between Linux and BSD, move > > > into common directory. > > > Rename eal_debug.c to eal_common_debug.c > > > Makefile changes to reflect file move and name change. > > > Fix checkpatch warnings. > > > > > > Signed-off-by: Ravi Kerur > > The subject matter for all of the patches in the series should be more > > detailed. > > Just posting move EAL components to common directly isn't overly > > informative > > Neil > > > > > --- > > > app/test/test_debug.c | 1 - > > > lib/librte_eal/bsdapp/eal/Makefile | 2 +- > > > lib/librte_eal/bsdapp/eal/eal_debug.c | 113 > > ------------------------------ > > > lib/librte_eal/common/eal_common_debug.c | 106 > > ++++++++++++++++++++++++++++ > > > lib/librte_eal/common/include/rte_debug.h | 7 -- > > > lib/librte_eal/linuxapp/eal/Makefile | 2 +- > > > lib/librte_eal/linuxapp/eal/eal_debug.c | 113 > > ------------------------------ > > > 7 files changed, 108 insertions(+), 236 deletions(-) > > > delete mode 100644 lib/librte_eal/bsdapp/eal/eal_debug.c > > > create mode 100644 lib/librte_eal/common/eal_common_debug.c > > > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_debug.c > > > > > > diff --git a/app/test/test_debug.c b/app/test/test_debug.c > > > index 7c3ee92..01d4b76 100644 > > > --- a/app/test/test_debug.c > > > +++ b/app/test/test_debug.c > > > @@ -136,7 +136,6 @@ static int > > > test_debug(void) > > > { > > > rte_dump_stack(); > > > - rte_dump_registers(); > > > if (test_panic() < 0) > > > return -1; > > > if (test_exit() < 0) > > > diff --git a/lib/librte_eal/bsdapp/eal/Makefile > > b/lib/librte_eal/bsdapp/eal/Makefile > > > index d434882..9b83e11 100644 > > > --- a/lib/librte_eal/bsdapp/eal/Makefile > > > +++ b/lib/librte_eal/bsdapp/eal/Makefile > > > @@ -53,7 +53,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += > > eal_hugepage_info.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_thread.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_log.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_pci.c > > > -SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_debug.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_lcore.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_timer.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_interrupts.c > > > @@ -73,6 +72,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += > > eal_common_hexdump.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_devargs.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_dev.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_options.c > > > +SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_debug.c > > > > > > CFLAGS_eal.o := -D_GNU_SOURCE > > > #CFLAGS_eal_thread.o := -D_GNU_SOURCE > > > diff --git a/lib/librte_eal/bsdapp/eal/eal_debug.c > > b/lib/librte_eal/bsdapp/eal/eal_debug.c > > > deleted file mode 100644 > > > index 44fc4f3..0000000 > > > --- a/lib/librte_eal/bsdapp/eal/eal_debug.c > > > +++ /dev/null > > > @@ -1,113 +0,0 @@ > > > -/*- > > > - * BSD LICENSE > > > - * > > > - * Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > > > - * All rights reserved. > > > - * > > > - * Redistribution and use in source and binary forms, with or without > > > - * modification, are permitted provided that the following conditions > > > - * are met: > > > - * > > > - * * Redistributions of source code must retain the above copyright > > > - * notice, this list of conditions and the following disclaimer. > > > - * * Redistributions in binary form must reproduce the above > > copyright > > > - * notice, this list of conditions and the following disclaimer in > > > - * the documentation and/or other materials provided with the > > > - * distribution. > > > - * * Neither the name of Intel Corporation nor the names of its > > > - * contributors may be used to endorse or promote products derived > > > - * from this software without specific prior written permission. > > > - * > > > - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > > - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > > - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS > > FOR > > > - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > > COPYRIGHT > > > - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, > > INCIDENTAL, > > > - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > > - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF > > USE, > > > - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON > > ANY > > > - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > > - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE > > USE > > > - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > DAMAGE. > > > - */ > > > - > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > - > > > -#include > > > -#include > > > -#include > > > - > > > -#define BACKTRACE_SIZE 256 > > > - > > > -/* dump the stack of the calling core */ > > > -void rte_dump_stack(void) > > > -{ > > > - void *func[BACKTRACE_SIZE]; > > > - char **symb = NULL; > > > - int size; > > > - > > > - size = backtrace(func, BACKTRACE_SIZE); > > > - symb = backtrace_symbols(func, size); > > > - while (size > 0) { > > > - rte_log(RTE_LOG_ERR, RTE_LOGTYPE_EAL, > > > - "%d: [%s]\n", size, symb[size - 1]); > > > - size --; > > > - } > > > -} > > > - > > > -/* not implemented in this environment */ > > > -void rte_dump_registers(void) > > > -{ > > > - return; > > > -} > > > - > > > -/* call abort(), it will generate a coredump if enabled */ > > > -void __rte_panic(const char *funcname, const char *format, ...) > > > -{ > > > - va_list ap; > > > - > > > - /* disable history */ > > > - rte_log_set_history(0); > > > - > > > - rte_log(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, "PANIC in %s():\n", > > funcname); > > > - va_start(ap, format); > > > - rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap); > > > - va_end(ap); > > > - rte_dump_stack(); > > > - rte_dump_registers(); > > > - abort(); > > > -} > > > - > > > -/* > > > - * Like rte_panic this terminates the application. However, no > > traceback is > > > - * provided and no core-dump is generated. > > > - */ > > > -void > > > -rte_exit(int exit_code, const char *format, ...) > > > -{ > > > - va_list ap; > > > - > > > - /* disable history */ > > > - rte_log_set_history(0); > > > - > > > - if (exit_code != 0) > > > - RTE_LOG(CRIT, EAL, "Error - exiting with code: %d\n" > > > - " Cause: ", exit_code); > > > - > > > - va_start(ap, format); > > > - rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap); > > > - va_end(ap); > > > - > > > -#ifndef RTE_EAL_ALWAYS_PANIC_ON_ERROR > > > - exit(exit_code); > > > -#else > > > - rte_dump_stack(); > > > - rte_dump_registers(); > > > - abort(); > > > -#endif > > > -} > > > diff --git a/lib/librte_eal/common/eal_common_debug.c > > b/lib/librte_eal/common/eal_common_debug.c > > > new file mode 100644 > > > index 0000000..9c10ee4 > > > --- /dev/null > > > +++ b/lib/librte_eal/common/eal_common_debug.c > > > @@ -0,0 +1,106 @@ > > > +/*- > > > + * BSD LICENSE > > > + * > > > + * Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > > > + * All rights reserved. > > > + * > > > + * Redistribution and use in source and binary forms, with or without > > > + * modification, are permitted provided that the following conditions > > > + * are met: > > > + * > > > + * * Redistributions of source code must retain the above copyright > > > + * notice, this list of conditions and the following disclaimer. > > > + * * Redistributions in binary form must reproduce the above > > copyright > > > + * notice, this list of conditions and the following disclaimer in > > > + * the documentation and/or other materials provided with the > > > + * distribution. > > > + * * Neither the name of Intel Corporation nor the names of its > > > + * contributors may be used to endorse or promote products derived > > > + * from this software without specific prior written permission. > > > + * > > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS > > FOR > > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > > COPYRIGHT > > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, > > INCIDENTAL, > > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF > > USE, > > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON > > ANY > > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE > > USE > > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > DAMAGE. > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#include > > > +#include > > > +#include > > > + > > > +#define BACKTRACE_SIZE 256 > > > + > > > +/* dump the stack of the calling core */ > > > +void rte_dump_stack(void) > > > +{ > > > + void *func[BACKTRACE_SIZE]; > > > + char **symb = NULL; > > > + int size; > > > + > > > + size = backtrace(func, BACKTRACE_SIZE); > > > + symb = backtrace_symbols(func, size); > > > + while (size > 0) { > > > + rte_log(RTE_LOG_ERR, RTE_LOGTYPE_EAL, > > > + "%d: [%s]\n", size, symb[size - 1]); > > > + size--; > > > + } > > > +} > > > + > > > +/* call abort(), it will generate a coredump if enabled */ > > > +void __rte_panic(const char *funcname, const char *format, ...) > > > +{ > > > + va_list ap; > > > + > > > + /* disable history */ > > > + rte_log_set_history(0); > > > + > > > + rte_log(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, "PANIC in %s():\n", > > funcname); > > > + va_start(ap, format); > > > + rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap); > > > + va_end(ap); > > > + rte_dump_stack(); > > > + abort(); > > > +} > > > + > > > +/* > > > + * Like rte_panic this terminates the application. However, no > > traceback is > > > + * provided and no core-dump is generated. if > > RTE_EAL_ALWAYS_PANIC_ON_ERROR > > > + * is not defined. > > > + */ > > > +void > > > +rte_exit(int exit_code, const char *format, ...) > > > +{ > > > + va_list ap; > > > + > > > + /* disable history */ > > > + rte_log_set_history(0); > > > + > > > + if (exit_code != 0) > > > + RTE_LOG(CRIT, EAL, "Error - exiting with code: %d\n" > > > + " Cause: ", exit_code); > > > + > > > + va_start(ap, format); > > > + rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap); > > > + va_end(ap); > > > + > > > +#ifndef RTE_EAL_ALWAYS_PANIC_ON_ERROR > > > + exit(exit_code); > > > +#else > > > + rte_dump_stack(); > > > + abort(); > > > +#endif > > > +} > > > diff --git a/lib/librte_eal/common/include/rte_debug.h > > b/lib/librte_eal/common/include/rte_debug.h > > > index 82ee3b3..3fb0307 100644 > > > --- a/lib/librte_eal/common/include/rte_debug.h > > > +++ b/lib/librte_eal/common/include/rte_debug.h > > > @@ -53,13 +53,6 @@ extern "C" { > > > void rte_dump_stack(void); > > > > > > /** > > > - * Dump the registers of the calling core to the console. > > > - * > > > - * Note: Not implemented in a userapp environment; use gdb instead. > > > - */ > > > -void rte_dump_registers(void); > > > - > > > -/** > > > * Provide notification of a critical non-recoverable error and > > terminate > > > * execution abnormally. > > > * > > > diff --git a/lib/librte_eal/linuxapp/eal/Makefile > > b/lib/librte_eal/linuxapp/eal/Makefile > > > index 72ecf3a..87b9bfc 100644 > > > --- a/lib/librte_eal/linuxapp/eal/Makefile > > > +++ b/lib/librte_eal/linuxapp/eal/Makefile > > > @@ -62,7 +62,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_uio.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_vfio.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_vfio_mp_sync.c > > > -SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_debug.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_lcore.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_timer.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_interrupts.c > > > @@ -85,6 +84,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += > > eal_common_hexdump.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_devargs.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_dev.c > > > SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_options.c > > > +SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_debug.c > > > > > > CFLAGS_eal.o := -D_GNU_SOURCE > > > CFLAGS_eal_thread.o := -D_GNU_SOURCE > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_debug.c > > b/lib/librte_eal/linuxapp/eal/eal_debug.c > > > deleted file mode 100644 > > > index 44fc4f3..0000000 > > > --- a/lib/librte_eal/linuxapp/eal/eal_debug.c > > > +++ /dev/null > > > @@ -1,113 +0,0 @@ > > > -/*- > > > - * BSD LICENSE > > > - * > > > - * Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > > > - * All rights reserved. > > > - * > > > - * Redistribution and use in source and binary forms, with or without > > > - * modification, are permitted provided that the following conditions > > > - * are met: > > > - * > > > - * * Redistributions of source code must retain the above copyright > > > - * notice, this list of conditions and the following disclaimer. > > > - * * Redistributions in binary form must reproduce the above > > copyright > > > - * notice, this list of conditions and the following disclaimer in > > > - * the documentation and/or other materials provided with the > > > - * distribution. > > > - * * Neither the name of Intel Corporation nor the names of its > > > - * contributors may be used to endorse or promote products derived > > > - * from this software without specific prior written permission. > > > - * > > > - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > > - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > > - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS > > FOR > > > - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > > COPYRIGHT > > > - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, > > INCIDENTAL, > > > - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > > - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF > > USE, > > > - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON > > ANY > > > - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > > - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE > > USE > > > - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > DAMAGE. > > > - */ > > > - > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > -#include > > > - > > > -#include > > > -#include > > > -#include > > > - > > > -#define BACKTRACE_SIZE 256 > > > - > > > -/* dump the stack of the calling core */ > > > -void rte_dump_stack(void) > > > -{ > > > - void *func[BACKTRACE_SIZE]; > > > - char **symb = NULL; > > > - int size; > > > - > > > - size = backtrace(func, BACKTRACE_SIZE); > > > - symb = backtrace_symbols(func, size); > > > - while (size > 0) { > > > - rte_log(RTE_LOG_ERR, RTE_LOGTYPE_EAL, > > > - "%d: [%s]\n", size, symb[size - 1]); > > > - size --; > > > - } > > > -} > > > - > > > -/* not implemented in this environment */ > > > -void rte_dump_registers(void) > > > -{ > > > - return; > > > -} > > > - > > > -/* call abort(), it will generate a coredump if enabled */ > > > -void __rte_panic(const char *funcname, const char *format, ...) > > > -{ > > > - va_list ap; > > > - > > > - /* disable history */ > > > - rte_log_set_history(0); > > > - > > > - rte_log(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, "PANIC in %s():\n", > > funcname); > > > - va_start(ap, format); > > > - rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap); > > > - va_end(ap); > > > - rte_dump_stack(); > > > - rte_dump_registers(); > > > - abort(); > > > -} > > > - > > > -/* > > > - * Like rte_panic this terminates the application. However, no > > traceback is > > > - * provided and no core-dump is generated. > > > - */ > > > -void > > > -rte_exit(int exit_code, const char *format, ...) > > > -{ > > > - va_list ap; > > > - > > > - /* disable history */ > > > - rte_log_set_history(0); > > > - > > > - if (exit_code != 0) > > > - RTE_LOG(CRIT, EAL, "Error - exiting with code: %d\n" > > > - " Cause: ", exit_code); > > > - > > > - va_start(ap, format); > > > - rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap); > > > - va_end(ap); > > > - > > > -#ifndef RTE_EAL_ALWAYS_PANIC_ON_ERROR > > > - exit(exit_code); > > > -#else > > > - rte_dump_stack(); > > > - rte_dump_registers(); > > > - abort(); > > > -#endif > > > -} > > > -- > > > 1.9.1 > > > > > > > >