Re: [dpdk-dev] [PATCH] eal: move rte_cpu definitions to os specific files

[David Marchand <david.marchand@redhat.com>]

On Mon, Sep 30, 2019 at 7:35 PM Pallavi Kadam <pallavi.kadam@intel.com> wrote:
>
> Moving RTE_CPU* definitions from the common code to the
> Linux, FreeBSD rte_os.h file to avoid #ifdef clutter.
>
> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
> Signed-off-by: Antara Ganesh Kolar <antara.ganesh.kolar@intel.com>
> Reviewed-by: Ranjit Menon <ranjit.menon@intel.com>

Took into account Jerin suggestion for title.

Reviewed-by: Jerin Jacob <jerinj@marvell.com>

> ---
>  lib/librte_eal/common/include/rte_lcore.h   | 44 ---------------------
>  lib/librte_eal/freebsd/eal/include/rte_os.h | 27 +++++++++++++
>  lib/librte_eal/linux/eal/include/rte_os.h   | 19 +++++++++
>  3 files changed, 46 insertions(+), 44 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
> index c86f72eb1..63ad4af13 100644
> --- a/lib/librte_eal/common/include/rte_lcore.h
> +++ b/lib/librte_eal/common/include/rte_lcore.h
> @@ -22,50 +22,6 @@ extern "C" {
>
>  #define LCORE_ID_ANY     UINT32_MAX       /**< Any lcore. */
>
> -#if defined(__linux__)
> -typedef        cpu_set_t rte_cpuset_t;
> -#define RTE_CPU_AND(dst, src1, src2) CPU_AND(dst, src1, src2)
> -#define RTE_CPU_OR(dst, src1, src2) CPU_OR(dst, src1, src2)
> -#define RTE_CPU_FILL(set) do \
> -{ \
> -       unsigned int i; \
> -       CPU_ZERO(set); \
> -       for (i = 0; i < CPU_SETSIZE; i++) \
> -               CPU_SET(i, set); \
> -} while (0)
> -#define RTE_CPU_NOT(dst, src) do \
> -{ \
> -       cpu_set_t tmp; \
> -       RTE_CPU_FILL(&tmp); \
> -       CPU_XOR(dst, &tmp, src); \
> -} while (0)
> -#elif defined(__FreeBSD__)
> -#include <pthread_np.h>
> -typedef cpuset_t rte_cpuset_t;
> -#define RTE_CPU_AND(dst, src1, src2) do \
> -{ \
> -       cpuset_t tmp; \
> -       CPU_COPY(src1, &tmp); \
> -       CPU_AND(&tmp, src2); \
> -       CPU_COPY(&tmp, dst); \
> -} while (0)
> -#define RTE_CPU_OR(dst, src1, src2) do \
> -{ \
> -       cpuset_t tmp; \
> -       CPU_COPY(src1, &tmp); \
> -       CPU_OR(&tmp, src2); \
> -       CPU_COPY(&tmp, dst); \
> -} while (0)
> -#define RTE_CPU_FILL(set) CPU_FILL(set)
> -#define RTE_CPU_NOT(dst, src) do \
> -{ \
> -       cpuset_t tmp; \
> -       CPU_FILL(&tmp); \
> -       CPU_NAND(&tmp, src); \
> -       CPU_COPY(&tmp, dst); \
> -} while (0)
> -#endif
> -
>  /**
>   * Structure storing internal configuration (per-lcore)
>   */
> diff --git a/lib/librte_eal/freebsd/eal/include/rte_os.h b/lib/librte_eal/freebsd/eal/include/rte_os.h
> index 49cd4d4d9..b414f6989 100644
> --- a/lib/librte_eal/freebsd/eal/include/rte_os.h
> +++ b/lib/librte_eal/freebsd/eal/include/rte_os.h
> @@ -11,4 +11,31 @@
>   * freebsd OS. Functions will be added in future releases.
>   */
>
> +#include <rte_per_lcore.h>
> +#include <pthread_np.h>

Including pthread_np.h is enough.


> +
> +typedef cpuset_t rte_cpuset_t;
> +#define RTE_CPU_AND(dst, src1, src2) do \
> +{ \
> +       cpuset_t tmp; \
> +       CPU_COPY(src1, &tmp); \
> +       CPU_AND(&tmp, src2); \
> +       CPU_COPY(&tmp, dst); \
> +} while (0)
> +#define RTE_CPU_OR(dst, src1, src2) do \
> +{ \
> +       cpuset_t tmp; \
> +       CPU_COPY(src1, &tmp); \
> +       CPU_OR(&tmp, src2); \
> +       CPU_COPY(&tmp, dst); \
> +} while (0)
> +#define RTE_CPU_FILL(set) CPU_FILL(set)
> +#define RTE_CPU_NOT(dst, src) do \
> +{ \
> +       cpuset_t tmp; \
> +       CPU_FILL(&tmp); \
> +       CPU_NAND(&tmp, src); \
> +       CPU_COPY(&tmp, dst); \
> +} while (0)
> +
>  #endif /* _RTE_OS_H_ */
> diff --git a/lib/librte_eal/linux/eal/include/rte_os.h b/lib/librte_eal/linux/eal/include/rte_os.h
> index bc6ad14d2..dd487a830 100644
> --- a/lib/librte_eal/linux/eal/include/rte_os.h
> +++ b/lib/librte_eal/linux/eal/include/rte_os.h
> @@ -11,4 +11,23 @@
>   * linux OS. Functions will be added in future releases.
>   */
>
> +#include <rte_per_lcore.h>

cpu_set_t and the CPU macros come from sched.h.
No need for rte_per_lcore.h.


> +
> +typedef        cpu_set_t rte_cpuset_t;

Fixed indent.


> +#define RTE_CPU_AND(dst, src1, src2) CPU_AND(dst, src1, src2)
> +#define RTE_CPU_OR(dst, src1, src2) CPU_OR(dst, src1, src2)
> +#define RTE_CPU_FILL(set) do \
> +{ \
> +       unsigned int i; \
> +       CPU_ZERO(set); \
> +       for (i = 0; i < CPU_SETSIZE; i++) \
> +               CPU_SET(i, set); \
> +} while (0)
> +#define RTE_CPU_NOT(dst, src) do \
> +{ \
> +       cpu_set_t tmp; \
> +       RTE_CPU_FILL(&tmp); \
> +       CPU_XOR(dst, &tmp, src); \
> +} while (0)
> +
>  #endif /* _RTE_OS_H_ */
> --
> 2.18.0.windows.1
>

Applied, thanks.



--
David Marchand