From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ilya Matveychikov <matvejchikov@gmail.com>
Subject: Re: [PATCH v3] mbuf: cleanup rte_pktmbuf_lastseg(),
 fix atomic usage
Date: Thu, 16 Nov 2017 12:07:01 +0400
Message-ID: <47A6864E-02DC-4050-9C12-E76F8FAA4F5D@gmail.com>
References: <20171115091413.27119-1-hhaim@cisco.com>
 <1D98684F-B8A9-4037-8534-0D4E3A1FD34C@gmail.com>
 <ad03f3a3142141afb9dc565500429df9@XCH-RTP-017.cisco.com>
 <20171115173058.mrkrv3usbl5sfw3h@platinum>
 <2fa9a7806c9e447995d6017c6def9894@XCH-RTP-017.cisco.com>
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Cc: Olivier MATZ <olivier.matz@6wind.com>,
 Konstantin Ananyev <konstantin.ananyev@intel.com>,
 "dev@dpdk.org" <dev@dpdk.org>
To: "Hanoch Haim (hhaim)" <hhaim@cisco.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68])
 by dpdk.org (Postfix) with ESMTP id F24331B22E
 for <dev@dpdk.org>; Thu, 16 Nov 2017 09:07:01 +0100 (CET)
Received: by mail-wm0-f68.google.com with SMTP id r68so7619001wmr.1
 for <dev@dpdk.org>; Thu, 16 Nov 2017 00:07:01 -0800 (PST)
In-Reply-To: <2fa9a7806c9e447995d6017c6def9894@XCH-RTP-017.cisco.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>


> On Nov 16, 2017, at 11:16 AM, Hanoch Haim (hhaim) <hhaim@cisco.com> =
wrote:
>=20
> Hi Oliver,=20
>=20
> It's hard for me to follow this thread.=20
>=20
> 1)  It is not about clear/not-clear, it is error prone to *replicate* =
code that has the same logic.
>=20
> "I'm not convinced that:
>=20
>    __rte_pktmbuf_reset_nb_segs(m);
>=20
> is clearer than:
>=20
>   m->next =3D NULL;
>   m->nb_segs =3D 1;
>=20
> Anyway, I agree this should not be part of this patch. We should only =
keep the fix.
> "
> 2) This definitely does not look good.=20
> All the point in my patch is to move the ref-cnt operations to set of =
API that already taking care of RTE_MBUF_REFCNT_ATOMIC
>=20
> +               /* We don't use rte_mbuf_refcnt_update() because we =
already
> +                * tested that refcnt !=3D 1.
> +                */
> +#ifdef RTE_MBUF_REFCNT_ATOMIC
> +               ret =3D rte_atomic16_add_return(&m->refcnt_atomic, =
-1);=20
> +#else
> +               ret =3D --m->refcnt;
> +#endif
> +               if (ret !=3D 0)
> +                       return NULL;
>=20

Looks ugly, agreed.