[v5,2/2] doc: announce deprecation of refcnt atomic member
Checks
Commit Message
refcnt_atomic member in structures rte_mbuf and rte_mbuf_ext_shared_info
will be deprecated in 20.11 release.
Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Phil Yang <phil.yang@arm.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
doc/guides/rel_notes/deprecation.rst | 6 ++++++
1 file changed, 6 insertions(+)
Comments
On Fri, Jul 17, 2020 at 12:36:51PM +0800, Phil Yang wrote:
> refcnt_atomic member in structures rte_mbuf and rte_mbuf_ext_shared_info
> will be deprecated in 20.11 release.
>
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
On Fri, Jul 17, 2020 at 6:37 AM Phil Yang <phil.yang@arm.com> wrote:
>
> refcnt_atomic member in structures rte_mbuf and rte_mbuf_ext_shared_info
> will be deprecated in 20.11 release.
>
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> doc/guides/rel_notes/deprecation.rst | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index a58a179..99c9806 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -129,6 +129,12 @@ Deprecation Notices
> in "rte_sched.h". These changes are aligned to improvements suggested in the
> RFC https://mails.dpdk.org/archives/dev/2018-November/120035.html.
>
> +* mbuf: ``refcnt_atomic`` member in structures ``rte_mbuf`` and
> + ``rte_mbuf_ext_shared_info`` is of type ``rte_atomic16_t``. Due to adoption
> + of C11 atomic builtins it will be of type ``uint16_t``. ``refcnt_atomic``
> + will be removed in 20.11. It will be replaced with ``refcnt`` of type
> + ``uint16_t``.
> +
> * metrics: The function ``rte_metrics_init`` will have a non-void return
> in order to notify errors instead of calling ``rte_exit``.
>
> --
> 2.7.4
>
Acked-by: David Marchand <david.marchand@redhat.com>
On Fri, Jul 17, 2020 at 4:32 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Fri, Jul 17, 2020 at 6:37 AM Phil Yang <phil.yang@arm.com> wrote:
> >
> > refcnt_atomic member in structures rte_mbuf and rte_mbuf_ext_shared_info
> > will be deprecated in 20.11 release.
> >
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > doc/guides/rel_notes/deprecation.rst | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index a58a179..99c9806 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -129,6 +129,12 @@ Deprecation Notices
> > in "rte_sched.h". These changes are aligned to improvements suggested in the
> > RFC https://mails.dpdk.org/archives/dev/2018-November/120035.html.
> >
> > +* mbuf: ``refcnt_atomic`` member in structures ``rte_mbuf`` and
> > + ``rte_mbuf_ext_shared_info`` is of type ``rte_atomic16_t``. Due to adoption
> > + of C11 atomic builtins it will be of type ``uint16_t``. ``refcnt_atomic``
> > + will be removed in 20.11. It will be replaced with ``refcnt`` of type
> > + ``uint16_t``.
> > +
> > * metrics: The function ``rte_metrics_init`` will have a non-void return
> > in order to notify errors instead of calling ``rte_exit``.
> >
> > --
> > 2.7.4
> >
>
> Acked-by: David Marchand <david.marchand@redhat.com>
Bruce, Konstantin,
This precedes the first open source release so trying with you guys:
what is the purpose of the RTE_MBUF_REFCNT_ATOMIC build flag?
Thanks.
Hi David,
> On Fri, Jul 17, 2020 at 4:32 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Fri, Jul 17, 2020 at 6:37 AM Phil Yang <phil.yang@arm.com> wrote:
> > >
> > > refcnt_atomic member in structures rte_mbuf and rte_mbuf_ext_shared_info
> > > will be deprecated in 20.11 release.
> > >
> > > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > > ---
> > > doc/guides/rel_notes/deprecation.rst | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > > index a58a179..99c9806 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -129,6 +129,12 @@ Deprecation Notices
> > > in "rte_sched.h". These changes are aligned to improvements suggested in the
> > > RFC https://mails.dpdk.org/archives/dev/2018-November/120035.html.
> > >
> > > +* mbuf: ``refcnt_atomic`` member in structures ``rte_mbuf`` and
> > > + ``rte_mbuf_ext_shared_info`` is of type ``rte_atomic16_t``. Due to adoption
> > > + of C11 atomic builtins it will be of type ``uint16_t``. ``refcnt_atomic``
> > > + will be removed in 20.11. It will be replaced with ``refcnt`` of type
> > > + ``uint16_t``.
> > > +
> > > * metrics: The function ``rte_metrics_init`` will have a non-void return
> > > in order to notify errors instead of calling ``rte_exit``.
> > >
> > > --
> > > 2.7.4
> > >
> >
> > Acked-by: David Marchand <david.marchand@redhat.com>
>
> Bruce, Konstantin,
>
> This precedes the first open source release so trying with you guys:
> what is the purpose of the RTE_MBUF_REFCNT_ATOMIC build flag?
> Thanks.
>
As I remember it was possible to build/run DPDK with non-atomic refcnt,
i.e. each lcore manages/works with its own mempools.
Don't know does anyone yet rely on that feature.
On Fri, Jul 17, 2020 at 04:35:56PM +0200, David Marchand wrote:
> On Fri, Jul 17, 2020 at 4:32 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Fri, Jul 17, 2020 at 6:37 AM Phil Yang <phil.yang@arm.com> wrote:
> > >
> > > refcnt_atomic member in structures rte_mbuf and rte_mbuf_ext_shared_info
> > > will be deprecated in 20.11 release.
> > >
> > > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > > ---
> > > doc/guides/rel_notes/deprecation.rst | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > > index a58a179..99c9806 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -129,6 +129,12 @@ Deprecation Notices
> > > in "rte_sched.h". These changes are aligned to improvements suggested in the
> > > RFC https://mails.dpdk.org/archives/dev/2018-November/120035.html.
> > >
> > > +* mbuf: ``refcnt_atomic`` member in structures ``rte_mbuf`` and
> > > + ``rte_mbuf_ext_shared_info`` is of type ``rte_atomic16_t``. Due to adoption
> > > + of C11 atomic builtins it will be of type ``uint16_t``. ``refcnt_atomic``
> > > + will be removed in 20.11. It will be replaced with ``refcnt`` of type
> > > + ``uint16_t``.
> > > +
> > > * metrics: The function ``rte_metrics_init`` will have a non-void return
> > > in order to notify errors instead of calling ``rte_exit``.
> > >
> > > --
> > > 2.7.4
> > >
> >
> > Acked-by: David Marchand <david.marchand@redhat.com>
>
> Bruce, Konstantin,
>
> This precedes the first open source release so trying with you guys:
> what is the purpose of the RTE_MBUF_REFCNT_ATOMIC build flag?
> Thanks.
>
That's indeed going back a long way!
IIRC When we first introduced a reference count, I believe we considered
cases where we would not need atomics to work on the ref count, and added
the build macro to remove the cost of the atomic in those instances. For
example, if a TCP stack wanted to hold on to an mbuf after transmission
rather than having it freed to the mempool (in case it needed to be
retransmitted), it could increment the reference count to ensure that
did not occur. Later if an ack for the TCP packet was received the buffer
could be freed. So long as the same thread that did the TX freed the buffer
later, no atomic increment or decrement was necessary.
Similarly for a run-to-completion app which fragmented packets using
multiple mbufs referencing a single packet buffer. So long as only a single
thread worked on the buffer, the reference counters need not be atomic.
In practice, the general case needs to be atomic ref-counts, and I'm not
sure who, if anyone, uses this setting in their apps. It should be
convertable to a runtime setting if needed.
/Bruce
On Fri, Jul 17, 2020 at 6:20 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Fri, Jul 17, 2020 at 04:35:56PM +0200, David Marchand wrote:
> > On Fri, Jul 17, 2020 at 4:32 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > On Fri, Jul 17, 2020 at 6:37 AM Phil Yang <phil.yang@arm.com> wrote:
> > > >
> > > > refcnt_atomic member in structures rte_mbuf and rte_mbuf_ext_shared_info
> > > > will be deprecated in 20.11 release.
> > > >
> > > > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > > > ---
> > > > doc/guides/rel_notes/deprecation.rst | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > > > index a58a179..99c9806 100644
> > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > @@ -129,6 +129,12 @@ Deprecation Notices
> > > > in "rte_sched.h". These changes are aligned to improvements suggested in the
> > > > RFC https://mails.dpdk.org/archives/dev/2018-November/120035.html.
> > > >
> > > > +* mbuf: ``refcnt_atomic`` member in structures ``rte_mbuf`` and
> > > > + ``rte_mbuf_ext_shared_info`` is of type ``rte_atomic16_t``. Due to adoption
> > > > + of C11 atomic builtins it will be of type ``uint16_t``. ``refcnt_atomic``
> > > > + will be removed in 20.11. It will be replaced with ``refcnt`` of type
> > > > + ``uint16_t``.
> > > > +
> > > > * metrics: The function ``rte_metrics_init`` will have a non-void return
> > > > in order to notify errors instead of calling ``rte_exit``.
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > >
> > > Acked-by: David Marchand <david.marchand@redhat.com>
> >
> > Bruce, Konstantin,
> >
> > This precedes the first open source release so trying with you guys:
> > what is the purpose of the RTE_MBUF_REFCNT_ATOMIC build flag?
> > Thanks.
> >
> That's indeed going back a long way!
>
> IIRC When we first introduced a reference count, I believe we considered
> cases where we would not need atomics to work on the ref count, and added
> the build macro to remove the cost of the atomic in those instances. For
> example, if a TCP stack wanted to hold on to an mbuf after transmission
> rather than having it freed to the mempool (in case it needed to be
> retransmitted), it could increment the reference count to ensure that
> did not occur. Later if an ack for the TCP packet was received the buffer
> could be freed. So long as the same thread that did the TX freed the buffer
> later, no atomic increment or decrement was necessary.
>
> Similarly for a run-to-completion app which fragmented packets using
> multiple mbufs referencing a single packet buffer. So long as only a single
> thread worked on the buffer, the reference counters need not be atomic.
>
> In practice, the general case needs to be atomic ref-counts, and I'm not
> sure who, if anyone, uses this setting in their apps. It should be
> convertable to a runtime setting if needed.
Thanks Bruce and Konstantin.
With the switch to meson and not being able to enable/disable this
build flag, we will lose this feature.
It seems to be a quite special use case, so we might need a new dedicated API?
>
> On Fri, Jul 17, 2020 at 6:20 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Fri, Jul 17, 2020 at 04:35:56PM +0200, David Marchand wrote:
> > > On Fri, Jul 17, 2020 at 4:32 PM David Marchand
> > > <david.marchand@redhat.com> wrote:
> > > >
> > > > On Fri, Jul 17, 2020 at 6:37 AM Phil Yang <phil.yang@arm.com> wrote:
> > > > >
> > > > > refcnt_atomic member in structures rte_mbuf and rte_mbuf_ext_shared_info
> > > > > will be deprecated in 20.11 release.
> > > > >
> > > > > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > > > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > > > > ---
> > > > > doc/guides/rel_notes/deprecation.rst | 6 ++++++
> > > > > 1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > > > > index a58a179..99c9806 100644
> > > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > > @@ -129,6 +129,12 @@ Deprecation Notices
> > > > > in "rte_sched.h". These changes are aligned to improvements suggested in the
> > > > > RFC https://mails.dpdk.org/archives/dev/2018-November/120035.html.
> > > > >
> > > > > +* mbuf: ``refcnt_atomic`` member in structures ``rte_mbuf`` and
> > > > > + ``rte_mbuf_ext_shared_info`` is of type ``rte_atomic16_t``. Due to adoption
> > > > > + of C11 atomic builtins it will be of type ``uint16_t``. ``refcnt_atomic``
> > > > > + will be removed in 20.11. It will be replaced with ``refcnt`` of type
> > > > > + ``uint16_t``.
> > > > > +
> > > > > * metrics: The function ``rte_metrics_init`` will have a non-void return
> > > > > in order to notify errors instead of calling ``rte_exit``.
> > > > >
> > > > > --
> > > > > 2.7.4
> > > > >
> > > >
> > > > Acked-by: David Marchand <david.marchand@redhat.com>
> > >
> > > Bruce, Konstantin,
> > >
> > > This precedes the first open source release so trying with you guys:
> > > what is the purpose of the RTE_MBUF_REFCNT_ATOMIC build flag?
> > > Thanks.
> > >
> > That's indeed going back a long way!
> >
> > IIRC When we first introduced a reference count, I believe we considered
> > cases where we would not need atomics to work on the ref count, and added
> > the build macro to remove the cost of the atomic in those instances. For
> > example, if a TCP stack wanted to hold on to an mbuf after transmission
> > rather than having it freed to the mempool (in case it needed to be
> > retransmitted), it could increment the reference count to ensure that
> > did not occur. Later if an ack for the TCP packet was received the buffer
> > could be freed. So long as the same thread that did the TX freed the buffer
> > later, no atomic increment or decrement was necessary.
> >
> > Similarly for a run-to-completion app which fragmented packets using
> > multiple mbufs referencing a single packet buffer. So long as only a single
> > thread worked on the buffer, the reference counters need not be atomic.
> >
> > In practice, the general case needs to be atomic ref-counts, and I'm not
> > sure who, if anyone, uses this setting in their apps. It should be
> > convertable to a runtime setting if needed.
>
> Thanks Bruce and Konstantin.
>
>
> With the switch to meson and not being able to enable/disable this
> build flag, we will lose this feature.
> It seems to be a quite special use case, so we might need a new dedicated API?
I'd better go for deprecate and remove it completely.
Konstantin
@@ -129,6 +129,12 @@ Deprecation Notices
in "rte_sched.h". These changes are aligned to improvements suggested in the
RFC https://mails.dpdk.org/archives/dev/2018-November/120035.html.
+* mbuf: ``refcnt_atomic`` member in structures ``rte_mbuf`` and
+ ``rte_mbuf_ext_shared_info`` is of type ``rte_atomic16_t``. Due to adoption
+ of C11 atomic builtins it will be of type ``uint16_t``. ``refcnt_atomic``
+ will be removed in 20.11. It will be replaced with ``refcnt`` of type
+ ``uint16_t``.
+
* metrics: The function ``rte_metrics_init`` will have a non-void return
in order to notify errors instead of calling ``rte_exit``.