From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2] bus/pci: update device devargs on each
	rescan
Date: Mon, 12 Nov 2018 01:47:49 +0100
Message-ID: <1656633.l3hu02U1SK@xps>
References: <20181105070447.67700-1-dariusz.stojaczyk@intel.com>
 <039ED4275CED7440929022BC67E70611532E04E5@SHSMSX103.ccr.corp.intel.com>
 <20181106234003.qhimuexm6z4mp7pr@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, =?ISO-8859-1?Q?Ga=EBtan?= Rivet <gaetan.rivet@6wind.com>,
 Dariusz Stojaczyk <darek.stojaczyk@gmail.com>
To: "Zhang, Qi Z" <qi.z.zhang@intel.com>, "Stojaczyk,
 Dariusz" <dariusz.stojaczyk@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com
 [64.147.123.25]) by dpdk.org (Postfix) with ESMTP id 5DDFF201
 for <dev@dpdk.org>; Mon, 12 Nov 2018 01:47:53 +0100 (CET)
In-Reply-To: <20181106234003.qhimuexm6z4mp7pr@bidouze.vm.6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

07/11/2018 00:40, Ga=EBtan Rivet:
> On Tue, Nov 06, 2018 at 10:21:38PM +0000, Zhang, Qi Z wrote:
> > From: Dariusz Stojaczyk [mailto:darek.stojaczyk@gmail.com]
> > > From: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> > >=20
> > > Bus rescan is done e.g. during the device hotplug, where devargs are
> > > re-allocated. By not updating the rte_device->devargs pointer we pote=
ntially
> > > make it a dangling one, as previous devargs could have been (or will =
be soon)
> > > freed.
> >=20
> > Yes, this is the similar issue we met on vdev.
> >=20
> > The key problem is we have rte_devargs_insert will destroy a devargs wh=
ich is still referenced by a rte_device
> > I'm thinking , why not just keep always keep a snapshot of devargs in r=
te_device to make things simple?
> > We could introduce a API like rte_dev_assign_devargs(dev, devargs) whic=
h handled the clone and destroy stuff and can be used for all buses.
> > If that idea is acceptable, I would prefer the issue in this patch coul=
d be fixed in pci_name_set by clone a new copy as workaround.
> >=20
>=20
> I agree that it would be useful to have
>=20
>     rte_devargs_map(devargs, device);
>=20
> That will link safely a devargs to a device, but cloning is overkill.
>=20
> I disagree that dangling pointers and loose references is fixed by
> introducing more cloning and copies of stuff here and there.
>=20
> We must tighten the device -> devargs coupling, not loosen it.

This issue is fixed with a different approach:
	http://git.dpdk.org/dpdk/commit/?id=3Dc7ad7754
	devargs: do not replace already inserted device