Re: [dpdk-dev] [PATCH v3] mbuf: cleanup rte_pktmbuf_lastseg(), fix atomic usage

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

Hi Olivier,

> I agree with Konstantin's comment done in another thread [1]:
> 
>   '''
>   That would cause extra read; cmp (and possible slowdown) for atomic refcnt.
>   If that really need to be fixed - probably we need to introduce a new function
>   that would do update without trying to read refctn first - rte_mbuf_refcnt_update_blind() or so.
>   '''
> 
> However I'm not sure a new function is really needed: the name is not ideal,

That was just the first one from top of my head  :)
If the issue in naming only - I suppose we can find something better.
Personally I like Ilya's one: __rte_mbuf_refcnt_update().

> and it would only be used once. What about the patch below?

It would do the job too, but looks a bit clumsy to me.
Konstantin

> 
> ==============================
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1361,8 +1361,18 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
> 
>                 return m;
> 
> -       } else if (rte_atomic16_add_return(&m->refcnt_atomic, -1) == 0) {
> +       } else {
> 
> +               /* We don't use rte_mbuf_refcnt_update() because we already
> +                * tested that refcnt != 1.
> +                */
> +#ifdef RTE_MBUF_REFCNT_ATOMIC
> +               ret = rte_atomic16_add_return(&m->refcnt_atomic, -1);
> +#else
> +               ret = --m->refcnt;
> +#endif
> +               if (ret != 0)
> +                       return NULL;
> 
>                 if (RTE_MBUF_INDIRECT(m))
>                         rte_pktmbuf_detach(m);
> @@ -1375,7 +1385,6 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
> 
>                 return m;
>         }
> -       return NULL;
>  }
> 
>  /* deprecated, replaced by rte_pktmbuf_prefree_seg() */
> ==============================
> 
> [1] http://dpdk.org/dev/patchwork/patch/31378/
> 
> 
> Regards,
> Olivier