Re: [PATCH v2] bus/pci: update device devargs on each rescan

[Thomas Monjalon <thomas@monjalon.net>]

07/11/2018 00:40, Gaëtan 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>
> > > 
> > > Bus rescan is done e.g. during the device hotplug, where devargs are
> > > re-allocated. By not updating the rte_device->devargs pointer we potentially
> > > make it a dangling one, as previous devargs could have been (or will be soon)
> > > freed.
> > 
> > Yes, this is the similar issue we met on vdev.
> > 
> > The key problem is we have rte_devargs_insert will destroy a devargs which is still referenced by a rte_device
> > I'm thinking , why not just keep always keep a snapshot of devargs in rte_device to make things simple?
> > We could introduce a API like rte_dev_assign_devargs(dev, devargs) which 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 could be fixed in pci_name_set by clone a new copy as workaround.
> > 
> 
> I agree that it would be useful to have
> 
>     rte_devargs_map(devargs, device);
> 
> That will link safely a devargs to a device, but cloning is overkill.
> 
> I disagree that dangling pointers and loose references is fixed by
> introducing more cloning and copies of stuff here and there.
> 
> 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=c7ad7754
	devargs: do not replace already inserted device