> -----Original Message-----
> From: Dariusz Stojaczyk [mailto:darek.stojac...@gmail.com]
> Sent: Monday, November 5, 2018 10:40 PM
> To: dev@dpdk.org; gaetan.ri...@6wind.com
> Cc: Stojaczyk, Dariusz <dariusz.stojac...@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>
> Subject: [PATCH v2] bus/pci: update device devargs on each rescan
> 
> From: Darek Stojaczyk <dariusz.stojac...@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.

> 
> Fixes: 55e411b301c3 ("bus/pci: fix resource mapping override")
> Cc: qi.z.zh...@intel.com
> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojac...@intel.com>
> ---
> Changes since v1:
>   * introduce additional pci_devargs_lookup() check to make
>     the code slightly more readable (Gaetan Rivet)
> 
>  drivers/bus/pci/linux/pci.c  |  5 +++++  drivers/bus/pci/pci_common.c |
> 3 ++-
>  drivers/bus/pci/private.h    | 11 +++++++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c index
> c99d523f0..a32d4848e 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -379,6 +379,11 @@ pci_scan_one(const char *dirname, const struct
> rte_pci_addr *addr)
>                                               RTE_LOG(ERR, EAL, "Unexpected 
> device scan
> at %s!\n",
>                                                       filename);
>                               }
> +
> +                             if (pci_devargs_lookup(dev2) !=
> +                                             dev2->device.devargs)
> +                                     pci_name_set(dev2);
> +
>                               free(dev);
>                       }
>                       return 0;
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index 6276e5d69..fcaf76ed2 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -42,7 +42,8 @@ const char *rte_pci_get_sysfs_path(void)
>       return path;
>  }
> 
> -static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
> +struct rte_devargs *
> +pci_devargs_lookup(struct rte_pci_device *dev)
>  {
>       struct rte_devargs *devargs;
>       struct rte_pci_addr addr;
> diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h index
> 13c3324bb..8b2e904f0 100644
> --- a/drivers/bus/pci/private.h
> +++ b/drivers/bus/pci/private.h
> @@ -43,6 +43,17 @@ void
>  pci_name_set(struct rte_pci_device *dev);
> 
>  /**
> + * Find matching up-to-date devargs for a given device.
> + *
> + * @param pci_dev
> + *   PCI device
> + * @return
> + *   A devargs pointer or NULL
> + */
> +struct rte_devargs *
> +pci_devargs_lookup(struct rte_pci_device *dev);
> +
> +/**
>   * Add a PCI device to the PCI Bus (append to PCI Device list). This function
>   * also updates the bus references of the PCI Device (and the generic device
>   * object embedded within.
> --
> 2.11.0

Reply via email to