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

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

On Tue, Nov 06, 2018 at 10:21:38PM +0000, Zhang, Qi Z wrote:
> 
> 
> > -----Original Message-----
> > From: Dariusz Stojaczyk [mailto:darek.stojaczyk@gmail.com]
> > Sent: Monday, November 5, 2018 10:40 PM
> > To: dev@dpdk.org; gaetan.rivet@6wind.com
> > Cc: Stojaczyk, Dariusz <dariusz.stojaczyk@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Subject: [PATCH v2] bus/pci: update device devargs on each rescan
> > 
> > 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.

-- 
Gaëtan Rivet
6WIND