mbox series

[0/3] add lock-free stack support discovery

Message ID 20210412082901.652736-1-kda@semihalf.com (mailing list archive)
Headers show
Series add lock-free stack support discovery | expand

Message

Stanislaw Kardach April 12, 2021, 8:28 a.m. UTC
The lock-free stack implementation (RTE_STACK_F_LF) is supported only on a
subset of platforms, namely x86_64 and arm64. Platforms supporting 128b atomics
have to opt-in to a generic or C11 implementations. All other platforms use a
stubbed implementation for push/pop operations which are basically NOPs.
However rte_stack_create() will not fail and application can proceed assuming
it has a working lock-free stack.

This means that among other things the stack_lf fast and perf tests will fail
as if implementation is wrong (which one can argue is). Therefore this patchset
tries to give user a way to check whether a lock_free is supported or not both
at compile time (build flag) and at runtime (ENOTSUP errno in rte_stack_create).

I have added cc to stable@dpdk.org because check-git-log.sh suggested it. I'm
not sure if adding a binary compatible change to API is worth stable@dpdk.org.

Cc: stable@dpdk.org

Stanislaw Kardach (3):
  stack: update lock-free supported archs
  stack: add compile flag for lock-free support
  test: run lock-free stack tests when supported

 app/test/test_stack.c                  | 4 ++++
 app/test/test_stack_perf.c             | 4 ++++
 doc/guides/rel_notes/release_21_05.rst | 4 ++++
 lib/librte_stack/rte_stack.c           | 4 +++-
 lib/librte_stack/rte_stack.h           | 3 ++-
 lib/librte_stack/rte_stack_lf.h        | 5 +++++
 6 files changed, 22 insertions(+), 2 deletions(-)

Comments

David Marchand April 16, 2021, 6:34 a.m. UTC | #1
On Mon, Apr 12, 2021 at 10:29 AM Stanislaw Kardach <kda@semihalf.com> wrote:
>
> The lock-free stack implementation (RTE_STACK_F_LF) is supported only on a
> subset of platforms, namely x86_64 and arm64. Platforms supporting 128b atomics
> have to opt-in to a generic or C11 implementations. All other platforms use a
> stubbed implementation for push/pop operations which are basically NOPs.
> However rte_stack_create() will not fail and application can proceed assuming
> it has a working lock-free stack.

Did you actually hit this issue or is this only theoretical?
I can only think of ppc64 displaying such behavior.
Stanislaw Kardach April 19, 2021, 2:44 p.m. UTC | #2
On Fri, Apr 16, 2021 at 08:34:29AM +0200, David Marchand wrote:
> On Mon, Apr 12, 2021 at 10:29 AM Stanislaw Kardach <kda@semihalf.com> wrote:
> >
> > The lock-free stack implementation (RTE_STACK_F_LF) is supported only on a
> > subset of platforms, namely x86_64 and arm64. Platforms supporting 128b atomics
> > have to opt-in to a generic or C11 implementations. All other platforms use a
> > stubbed implementation for push/pop operations which are basically NOPs.
> > However rte_stack_create() will not fail and application can proceed assuming
> > it has a working lock-free stack.
>
> Did you actually hit this issue or is this only theoretical?
> I can only think of ppc64 displaying such behavior.
>
I actually hit this issue while working on a RISC-V port.
My reasoning here is that sooner or later someone else will stumble upon
this, either on ppc64 or while trying to port to some new platform.
It is also a really nasty limitation do debug given the silent nature of
the failure.

>
> --
> David Marchand
>

--
Best Regards,
Stanislaw Kardach
David Marchand May 3, 2021, 2:21 p.m. UTC | #3
On Mon, Apr 12, 2021 at 10:29 AM Stanislaw Kardach <kda@semihalf.com> wrote:
>
> The lock-free stack implementation (RTE_STACK_F_LF) is supported only on a
> subset of platforms, namely x86_64 and arm64. Platforms supporting 128b atomics
> have to opt-in to a generic or C11 implementations. All other platforms use a
> stubbed implementation for push/pop operations which are basically NOPs.
> However rte_stack_create() will not fail and application can proceed assuming
> it has a working lock-free stack.
>
> This means that among other things the stack_lf fast and perf tests will fail
> as if implementation is wrong (which one can argue is). Therefore this patchset
> tries to give user a way to check whether a lock_free is supported or not both
> at compile time (build flag) and at runtime (ENOTSUP errno in rte_stack_create).
>
> I have added cc to stable@dpdk.org because check-git-log.sh suggested it. I'm
> not sure if adding a binary compatible change to API is worth stable@dpdk.org.
>
> Cc: stable@dpdk.org

The issue was hit while porting to a new architecture.
The feature is broken in existing stable releases and it won't get
fixed by this change.

I'd rather not backport it.

Opinions?
Olivier Matz May 3, 2021, 2:28 p.m. UTC | #4
On Mon, May 03, 2021 at 04:21:25PM +0200, David Marchand wrote:
> On Mon, Apr 12, 2021 at 10:29 AM Stanislaw Kardach <kda@semihalf.com> wrote:
> >
> > The lock-free stack implementation (RTE_STACK_F_LF) is supported only on a
> > subset of platforms, namely x86_64 and arm64. Platforms supporting 128b atomics
> > have to opt-in to a generic or C11 implementations. All other platforms use a
> > stubbed implementation for push/pop operations which are basically NOPs.
> > However rte_stack_create() will not fail and application can proceed assuming
> > it has a working lock-free stack.
> >
> > This means that among other things the stack_lf fast and perf tests will fail
> > as if implementation is wrong (which one can argue is). Therefore this patchset
> > tries to give user a way to check whether a lock_free is supported or not both
> > at compile time (build flag) and at runtime (ENOTSUP errno in rte_stack_create).
> >
> > I have added cc to stable@dpdk.org because check-git-log.sh suggested it. I'm
> > not sure if adding a binary compatible change to API is worth stable@dpdk.org.
> >
> > Cc: stable@dpdk.org
> 
> The issue was hit while porting to a new architecture.
> The feature is broken in existing stable releases and it won't get
> fixed by this change.
> 
> I'd rather not backport it.
> 
> Opinions?

Agreed.
Stanislaw Kardach May 3, 2021, 6:34 p.m. UTC | #5
On Mon, 3 May 2021, 16:28 Olivier Matz, <olivier.matz@6wind.com> wrote:

> On Mon, May 03, 2021 at 04:21:25PM +0200, David Marchand wrote:
> > On Mon, Apr 12, 2021 at 10:29 AM Stanislaw Kardach <kda@semihalf.com>
> wrote:
> > >
> > > The lock-free stack implementation (RTE_STACK_F_LF) is supported only
> on a
> > > subset of platforms, namely x86_64 and arm64. Platforms supporting
> 128b atomics
> > > have to opt-in to a generic or C11 implementations. All other
> platforms use a
> > > stubbed implementation for push/pop operations which are basically
> NOPs.
> > > However rte_stack_create() will not fail and application can proceed
> assuming
> > > it has a working lock-free stack.
> > >
> > > This means that among other things the stack_lf fast and perf tests
> will fail
> > > as if implementation is wrong (which one can argue is). Therefore this
> patchset
> > > tries to give user a way to check whether a lock_free is supported or
> not both
> > > at compile time (build flag) and at runtime (ENOTSUP errno in
> rte_stack_create).
> > >
> > > I have added cc to stable@dpdk.org because check-git-log.sh suggested
> it. I'm
> > > not sure if adding a binary compatible change to API is worth
> stable@dpdk.org.
> > >
> > > Cc: stable@dpdk.org
> >
> > The issue was hit while porting to a new architecture.
> > The feature is broken in existing stable releases and it won't get
> > fixed by this change.
> >
> > I'd rather not backport it.
> >
> > Opinions?
>
> Agreed.
>
Agreed.

>
David Marchand May 4, 2021, 6:44 a.m. UTC | #6
On Mon, May 3, 2021 at 8:35 PM Stanisław Kardach <kda@semihalf.com> wrote:
> On Mon, 3 May 2021, 16:28 Olivier Matz, <olivier.matz@6wind.com> wrote:
>> On Mon, May 03, 2021 at 04:21:25PM +0200, David Marchand wrote:
>> > On Mon, Apr 12, 2021 at 10:29 AM Stanislaw Kardach <kda@semihalf.com> wrote:
>> > >
>> > > I have added cc to stable@dpdk.org because check-git-log.sh suggested it. I'm
>> > > not sure if adding a binary compatible change to API is worth stable@dpdk.org.
>> > >
>> > > Cc: stable@dpdk.org
>> >
>> > The issue was hit while porting to a new architecture.
>> > The feature is broken in existing stable releases and it won't get
>> > fixed by this change.
>> >
>> > I'd rather not backport it.
>> >
>> > Opinions?
>>
>> Agreed.
>
> Agreed.


Ok, thanks.
I'll take this series dropping Cc: stable.
David Marchand May 4, 2021, 6:44 a.m. UTC | #7
On Mon, Apr 12, 2021 at 10:29 AM Stanislaw Kardach <kda@semihalf.com> wrote:
>
> The lock-free stack implementation (RTE_STACK_F_LF) is supported only on a
> subset of platforms, namely x86_64 and arm64. Platforms supporting 128b atomics
> have to opt-in to a generic or C11 implementations. All other platforms use a
> stubbed implementation for push/pop operations which are basically NOPs.
> However rte_stack_create() will not fail and application can proceed assuming
> it has a working lock-free stack.
>
> This means that among other things the stack_lf fast and perf tests will fail
> as if implementation is wrong (which one can argue is). Therefore this patchset
> tries to give user a way to check whether a lock_free is supported or not both
> at compile time (build flag) and at runtime (ENOTSUP errno in rte_stack_create).

Series applied.
Thanks Stanislaw!