Re: [dpdk-dev] [PATCH v2 1/6] Move EAL common functions

[Ravi Kerur <rkerur@gmail.com>]

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??

Thanks.

On Sun, Dec 28, 2014 at 12:42 PM, Neil Horman <nhorman@tuxdriver.com> 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 <rkerur@gmail.com>
> 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 <execinfo.h>
> > -#include <stdarg.h>
> > -#include <signal.h>
> > -#include <stdlib.h>
> > -#include <stdio.h>
> > -#include <stdint.h>
> > -
> > -#include <rte_log.h>
> > -#include <rte_debug.h>
> > -#include <rte_common.h>
> > -
> > -#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 <execinfo.h>
> > +#include <stdarg.h>
> > +#include <signal.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <stdint.h>
> > +
> > +#include <rte_log.h>
> > +#include <rte_debug.h>
> > +#include <rte_common.h>
> > +
> > +#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 <execinfo.h>
> > -#include <stdarg.h>
> > -#include <signal.h>
> > -#include <stdlib.h>
> > -#include <stdio.h>
> > -#include <stdint.h>
> > -
> > -#include <rte_log.h>
> > -#include <rte_debug.h>
> > -#include <rte_common.h>
> > -
> > -#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
> >
> >
>