Re: [dpdk-dev] [PATCH v11 3/3] eal/stack: enable lock-free stack for aarch64

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

On Fri, Oct 18, 2019 at 1:22 PM Phil Yang <phil.yang@arm.com> wrote:
>
> Enable both c11 atomic and non c11 atomic lock-free stack for aarch64.
>
> Introduced a new header to reduce the ifdef clutter across generic and c11
> files. The rte_stack_lf_stubs.h contains stub implementations of
> __rte_stack_lf_count, __rte_stack_lf_push_elems and
> __rte_stack_lf_pop_elems.
>
> Suggested-by: Gage Eads <gage.eads@intel.com>
> Suggested-by: Jerin Jacob <jerinj@marvell.com>
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Tested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> ---
>  doc/guides/prog_guide/env_abstraction_layer.rst |  4 +--
>  doc/guides/rel_notes/release_19_11.rst          |  3 ++
>  lib/librte_stack/Makefile                       |  3 +-
>  lib/librte_stack/rte_stack_lf.h                 |  4 +++
>  lib/librte_stack/rte_stack_lf_c11.h             | 16 ---------
>  lib/librte_stack/rte_stack_lf_generic.h         | 16 ---------
>  lib/librte_stack/rte_stack_lf_stubs.h           | 44 +++++++++++++++++++++++++
>  7 files changed, 55 insertions(+), 35 deletions(-)
>  create mode 100644 lib/librte_stack/rte_stack_lf_stubs.h
>
> diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst
> index 94f30fd..6e59fae 100644
> --- a/doc/guides/prog_guide/env_abstraction_layer.rst
> +++ b/doc/guides/prog_guide/env_abstraction_layer.rst
> @@ -648,8 +648,8 @@ Known Issues
>    Alternatively, applications can use the lock-free stack mempool handler. When
>    considering this handler, note that:
>
> -  - It is currently limited to the x86_64 platform, because it uses an
> -    instruction (16-byte compare-and-swap) that is not yet available on other
> +  - It is currently limited to the aarch64 and x86_64 platforms, because it uses
> +    an instruction (16-byte compare-and-swap) that is not yet available on other
>      platforms.
>    - It has worse average-case performance than the non-preemptive rte_ring, but
>      software caching (e.g. the mempool cache) can mitigate this by reducing the
> diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> index 85953b9..4f82f54 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -115,6 +115,9 @@ New Features
>    Added eBPF JIT support for arm64 architecture to improve the eBPF program
>    performance.
>
> +* **Added Lock-free Stack for aarch64.**
> +
> +  The lock-free stack implementation is enabled for aarch64 platforms.

Missing line (paragraphs are separated with two empty lines).
This entry should go with Core libs entries, so at the beginning of this list.

I did the changes.

>
>  Removed Items
>  -------------
> diff --git a/lib/librte_stack/Makefile b/lib/librte_stack/Makefile
> index 8d18ce5..b5e5bed 100644
> --- a/lib/librte_stack/Makefile
> +++ b/lib/librte_stack/Makefile
> @@ -24,6 +24,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_STACK)-include := rte_stack.h \
>                                               rte_stack_std.h \
>                                               rte_stack_lf.h \
>                                               rte_stack_lf_generic.h \
> -                                             rte_stack_lf_c11.h
> +                                             rte_stack_lf_c11.h \
> +                                             rte_stack_lf_stubs.h
>
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_stack/rte_stack_lf.h b/lib/librte_stack/rte_stack_lf.h
> index f5581f0..e67630c 100644
> --- a/lib/librte_stack/rte_stack_lf.h
> +++ b/lib/librte_stack/rte_stack_lf.h
> @@ -5,11 +5,15 @@
>  #ifndef _RTE_STACK_LF_H_
>  #define _RTE_STACK_LF_H_
>
> +#if !(defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_ARM64))
> +#include "rte_stack_lf_stubs.h"
> +#else
>  #ifdef RTE_USE_C11_MEM_MODEL
>  #include "rte_stack_lf_c11.h"
>  #else
>  #include "rte_stack_lf_generic.h"
>  #endif
> +#endif
>
>  /**
>   * @internal Push several objects on the lock-free stack (MT-safe).
> diff --git a/lib/librte_stack/rte_stack_lf_c11.h b/lib/librte_stack/rte_stack_lf_c11.h
> index 3d677ae..999359f 100644
> --- a/lib/librte_stack/rte_stack_lf_c11.h
> +++ b/lib/librte_stack/rte_stack_lf_c11.h
> @@ -36,12 +36,6 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
>                           struct rte_stack_lf_elem *last,
>                           unsigned int num)
>  {
> -#ifndef RTE_ARCH_X86_64
> -       RTE_SET_USED(first);
> -       RTE_SET_USED(last);
> -       RTE_SET_USED(list);
> -       RTE_SET_USED(num);
> -#else
>         struct rte_stack_lf_head old_head;
>         int success;
>
> @@ -79,7 +73,6 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
>          * to the LIFO len update.
>          */
>         __atomic_add_fetch(&list->len, num, __ATOMIC_RELEASE);
> -#endif
>  }
>
>  static __rte_always_inline struct rte_stack_lf_elem *
> @@ -88,14 +81,6 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
>                          void **obj_table,
>                          struct rte_stack_lf_elem **last)
>  {
> -#ifndef RTE_ARCH_X86_64
> -       RTE_SET_USED(obj_table);
> -       RTE_SET_USED(last);
> -       RTE_SET_USED(list);
> -       RTE_SET_USED(num);
> -
> -       return NULL;
> -#else
>         struct rte_stack_lf_head old_head;
>         uint64_t len;
>         int success;
> @@ -169,7 +154,6 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
>         } while (success == 0);
>
>         return old_head.top;
> -#endif
>  }
>
>  #endif /* _RTE_STACK_LF_C11_H_ */
> diff --git a/lib/librte_stack/rte_stack_lf_generic.h b/lib/librte_stack/rte_stack_lf_generic.h
> index 3182151..3abbb53 100644
> --- a/lib/librte_stack/rte_stack_lf_generic.h
> +++ b/lib/librte_stack/rte_stack_lf_generic.h
> @@ -36,12 +36,6 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
>                           struct rte_stack_lf_elem *last,
>                           unsigned int num)
>  {
> -#ifndef RTE_ARCH_X86_64
> -       RTE_SET_USED(first);
> -       RTE_SET_USED(last);
> -       RTE_SET_USED(list);
> -       RTE_SET_USED(num);
> -#else
>         struct rte_stack_lf_head old_head;
>         int success;
>
> @@ -75,7 +69,6 @@ __rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
>         } while (success == 0);
>
>         rte_atomic64_add((rte_atomic64_t *)&list->len, num);
> -#endif
>  }
>
>  static __rte_always_inline struct rte_stack_lf_elem *
> @@ -84,14 +77,6 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
>                          void **obj_table,
>                          struct rte_stack_lf_elem **last)
>  {
> -#ifndef RTE_ARCH_X86_64
> -       RTE_SET_USED(obj_table);
> -       RTE_SET_USED(last);
> -       RTE_SET_USED(list);
> -       RTE_SET_USED(num);
> -
> -       return NULL;
> -#else
>         struct rte_stack_lf_head old_head;
>         int success;
>
> @@ -159,7 +144,6 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
>         } while (success == 0);
>
>         return old_head.top;
> -#endif
>  }
>
>  #endif /* _RTE_STACK_LF_GENERIC_H_ */
> diff --git a/lib/librte_stack/rte_stack_lf_stubs.h b/lib/librte_stack/rte_stack_lf_stubs.h
> new file mode 100644
> index 0000000..a05abf1
> --- /dev/null
> +++ b/lib/librte_stack/rte_stack_lf_stubs.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2019 Arm Limited
> + */
> +
> +#ifndef _RTE_STACK_LF_STUBS_H_
> +#define _RTE_STACK_LF_STUBS_H_
> +
> +#include <rte_common.h>
> +
> +static __rte_always_inline unsigned int
> +__rte_stack_lf_count(struct rte_stack *s)
> +{
> +       RTE_SET_USED(s);
> +
> +       return 0;
> +}
> +
> +static __rte_always_inline void
> +__rte_stack_lf_push_elems(struct rte_stack_lf_list *list,
> +                         struct rte_stack_lf_elem *first,
> +                         struct rte_stack_lf_elem *last,
> +                         unsigned int num)
> +{
> +       RTE_SET_USED(first);
> +       RTE_SET_USED(last);
> +       RTE_SET_USED(list);
> +       RTE_SET_USED(num);
> +}
> +
> +static __rte_always_inline struct rte_stack_lf_elem *
> +__rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
> +                        unsigned int num,
> +                        void **obj_table,
> +                        struct rte_stack_lf_elem **last)
> +{
> +       RTE_SET_USED(obj_table);
> +       RTE_SET_USED(last);
> +       RTE_SET_USED(list);
> +       RTE_SET_USED(num);
> +
> +       return NULL;
> +}
> +
> +#endif /* _RTE_STACK_LF_STUBS_H_ */
> --
> 2.7.4
>

Applied, thanks.

-- 
David Marchand