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

[David Marchand <david.marchand@redhat.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?


-- 
David Marchand