Re: [dpdk-dev] [PATCH v5 2/2] doc: announce deprecation of refcnt atomic member

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

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