From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH 07/11] ethdev: add lock to port allocation
	check
Date: Wed, 09 May 2018 14:25:03 +0200
Message-ID: <2776783.KmHTlbbVux@xps>
References: <20180509094337.26112-1-thomas@monjalon.net>
 <20180509094337.26112-8-thomas@monjalon.net>
 <20180509122116.ljistjpz3dfljdqo@bidouze.vm.6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Cc: dev@dpdk.org, Matan Azrad <matan@mellanox.com>
To: =?ISO-8859-1?Q?Ga=EBtan?= Rivet <gaetan.rivet@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com
 [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 98EC77E93
 for <dev@dpdk.org>; Wed,  9 May 2018 14:25:05 +0200 (CEST)
In-Reply-To: <20180509122116.ljistjpz3dfljdqo@bidouze.vm.6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

09/05/2018 14:21, Ga=EBtan Rivet:
> Hi,
>=20
> On Wed, May 09, 2018 at 11:43:33AM +0200, Thomas Monjalon wrote:
> > From: Matan Azrad <matan@mellanox.com>
> >=20
> > When comparing the port name, there can be a race condition with
> > a thread allocating a new port and writing the name at the same time.
> > It can lead to match with a partial name by error.
> >=20
> > The check of the port is now considered as a critical section
> > protected with locks.
> >=20
> > This fix will be even more required for multi-process when the
> > port availability will rely only on the name, in a following patch.
> >=20
> > Fixes: 84934303a17c ("ethdev: synchronize port allocation")
> > Cc: stable@dpdk.org
> >=20
> > Signed-off-by: Matan Azrad <matan@mellanox.com>
> > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >  lib/librte_ethdev/rte_ethdev.c | 22 +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> >=20
> > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_eth=
dev.c
> > index ae86d0ba7..357be2dca 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -227,8 +227,8 @@ rte_eth_dev_shared_data_prepare(void)
> >  	rte_spinlock_unlock(&rte_eth_shared_data_lock);
> >  }
> > =20
> > -struct rte_eth_dev *
> > -rte_eth_dev_allocated(const char *name)
> > +static struct rte_eth_dev *
> > +rte_eth_dev_allocated_lock_free(const char *name)
>=20
> A suggestion about the naming here.
> Reading subsequent patches, we can see this function being used during
> ethdev allocation routines. The _lock_free suffix is a little
> misleading, as for an instant one can think that there is something
> being freed about an allocated ethdev lock.
>=20
> I would suggest
>=20
>   * rte_eth_dev_allocated_nolock
>     or
>   * rte_eth_dev_allocated_lockless
>     (or even rte_eth_lockless_dev_allocated)
>=20
> instead.

Good suggestions. I vote for rte_eth_dev_allocated_nolock.
Thanks