On Thursday 24 November 2016 01:37 AM, Ben Walker wrote: > Two functions is both confusing and unnecessary. Previously, > rte_eal_pci_scan populated an internal list of devices by > scanning sysfs. Then, rte_eal_pci_probe would match registered > drivers to that internal list. These are not really useful > operations to perform separately, though, so > simplify the api down to just rte_eal_pci_probe which can > be called repeatedly through the lifetime of the application > to scan for new or removed PCI devices and load or unload > drivers as required.
Agree with this. And similar case exists for rte_eal_dev_init() for which there is another patch floated by Jerin [1]. Also, I am already merging these two (EAL Bus model) [2]. So, we have: - Only a probe called from EAL for all devices, whether PCI, VDEV or another other type - Probe in turns performs all scans and driver->probes() Concern I have is that with the change of placement for device scan, would it impact the external modules/PMDs as currently the scanned list is created *before* eal_plugins_init(). But, now the list is created *after* plugin initialization. There are other similar inits like creation of slave threads. As well as concern raised by Ferruh about device enumeration. I don't have clear idea of all such dependencies. Maybe David and Ferruh in CC might be able to comment better. [1] http://dpdk.org/ml/archives/dev/2016-November/050415.html [2] http://dpdk.org/ml/archives/dev/2016-November/050301.html > > Signed-off-by: Ben Walker <benjamin.walker at intel.com> > --- > app/test/test_pci.c | 2 +- > lib/librte_eal/bsdapp/eal/eal.c | 3 --- > lib/librte_eal/bsdapp/eal/eal_pci.c | 17 +---------------- > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 - > lib/librte_eal/common/eal_common_pci.c | 6 ++++++ > lib/librte_eal/common/eal_private.h | 14 +++++--------- > lib/librte_eal/common/include/rte_pci.h | 17 +++++------------ > lib/librte_eal/linuxapp/eal/eal.c | 3 --- > lib/librte_eal/linuxapp/eal/eal_pci.c | 18 +----------------- > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 - > 10 files changed, 19 insertions(+), 63 deletions(-) > > diff --git a/app/test/test_pci.c b/app/test/test_pci.c > index cda186d..fdd84f7 100644 > --- a/app/test/test_pci.c > +++ b/app/test/test_pci.c > @@ -180,7 +180,7 @@ test_pci_setup(void) > TAILQ_INSERT_TAIL(&real_pci_device_list, dev, next); > } > > - ret = rte_eal_pci_scan(); > + ret = rte_eal_pci_probe(); > TEST_ASSERT_SUCCESS(ret, "failed to scan PCI bus"); > rte_eal_pci_dump(stdout); > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c > index 35e3117..fd44528 100644 > --- a/lib/librte_eal/bsdapp/eal/eal.c > +++ b/lib/librte_eal/bsdapp/eal/eal.c > @@ -561,9 +561,6 @@ rte_eal_init(int argc, char **argv) > if (rte_eal_timer_init() < 0) > rte_panic("Cannot init HPET or TSC timers\n"); > > - if (rte_eal_pci_init() < 0) > - rte_panic("Cannot init PCI\n"); > - > eal_check_mem_on_local_socket(); > > if (eal_plugins_init() < 0) > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c > b/lib/librte_eal/bsdapp/eal/eal_pci.c > index 8b3ed88..6c3a169 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c > @@ -361,7 +361,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf) > * list. Call pci_scan_one() for each pci entry found. > */ > int > -rte_eal_pci_scan(void) > +pci_scan(void) > { > int fd; > unsigned dev_count = 0; > @@ -667,18 +667,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p) > > return ret; > } > - > -/* Init the PCI EAL subsystem */ > -int > -rte_eal_pci_init(void) > -{ > - /* for debug purposes, PCI can be disabled */ > - if (internal_config.no_pci) > - return 0; > - > - if (rte_eal_pci_scan() < 0) { > - RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__); > - return -1; > - } > - return 0; > -} > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map > b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > index 2f81f7c..67c469c 100644 > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > @@ -44,7 +44,6 @@ DPDK_2.0 { > rte_eal_pci_probe; > rte_eal_pci_probe_one; > rte_eal_pci_register; > - rte_eal_pci_scan; > rte_eal_pci_unregister; > rte_eal_process_type; > rte_eal_remote_launch; > diff --git a/lib/librte_eal/common/eal_common_pci.c > b/lib/librte_eal/common/eal_common_pci.c > index 4f8c3a0..d50a534 100644 > --- a/lib/librte_eal/common/eal_common_pci.c > +++ b/lib/librte_eal/common/eal_common_pci.c > @@ -81,6 +81,7 @@ > #include <rte_devargs.h> > > #include "eal_private.h" > +#include "eal_internal_cfg.h" > > struct pci_driver_list pci_driver_list = > TAILQ_HEAD_INITIALIZER(pci_driver_list); > @@ -423,6 +424,11 @@ rte_eal_pci_probe(void) > int probe_all = 0; > int ret = 0; > > + if (internal_config.no_pci) > + return 0; > + > + pci_scan(); > + So, no check for error reported by scan? I think we should, 1) because pci_scan() returns one, and 2) because in case scan failed, there is no reason probe would do anything worthwhile. > if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0) > probe_all = 1; > > diff --git a/lib/librte_eal/common/eal_private.h > b/lib/librte_eal/common/eal_private.h > index 9e7d8f6..54f18ea 100644 > --- a/lib/librte_eal/common/eal_private.h > +++ b/lib/librte_eal/common/eal_private.h > @@ -108,18 +108,14 @@ int rte_eal_timer_init(void); > */ > int rte_eal_log_init(const char *id, int facility); > > -/** > - * Init the PCI infrastructure > +struct rte_pci_driver; > +struct rte_pci_device; > + > +/* Scan the PCI bus for devices > * > * This function is private to EAL. > - * > - * @return > - * 0 on success, negative on error > */ > -int rte_eal_pci_init(void); > - > -struct rte_pci_driver; > -struct rte_pci_device; > +int pci_scan(void); > > /** > * Update a pci device object by asking the kernel for the latest > information. > diff --git a/lib/librte_eal/common/include/rte_pci.h > b/lib/librte_eal/common/include/rte_pci.h > index 5d0feac..2154a54 100644 > --- a/lib/librte_eal/common/include/rte_pci.h > +++ b/lib/librte_eal/common/include/rte_pci.h > @@ -386,20 +386,13 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__) > void rte_eal_pci_unregister(struct rte_pci_driver *driver); > > /** > - * Scan the content of the PCI bus, and the devices in the devices > - * list > - * > - * @return > - * 0 on success, negative on error > - */ > -int rte_eal_pci_scan(void); > - > -/** > - * Probe the PCI bus for registered drivers. > + * Scan the PCI bus for devices and match them to their driver. > * > * Scan the content of the PCI bus, and call the probe() function for > - * all registered drivers that have a matching entry in its id_table > - * for discovered devices. > + * all registered drivers that have a matching entry in their id_table. > + * If a device already has a driver loaded, probe will not be called. > + * If a previously discovered device is no longer present on the system, > + * the associated driver's remove() callback will be called. > * > * @return > * - 0 on success. > diff --git a/lib/librte_eal/linuxapp/eal/eal.c > b/lib/librte_eal/linuxapp/eal/eal.c > index 2075282..f47f361 100644 > --- a/lib/librte_eal/linuxapp/eal/eal.c > +++ b/lib/librte_eal/linuxapp/eal/eal.c > @@ -802,9 +802,6 @@ rte_eal_init(int argc, char **argv) > if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0) > rte_panic("Cannot init logs\n"); > > - if (rte_eal_pci_init() < 0) > - rte_panic("Cannot init PCI\n"); > - > #ifdef VFIO_PRESENT > if (rte_eal_vfio_setup() < 0) > rte_panic("Cannot init VFIO\n"); > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c > b/lib/librte_eal/linuxapp/eal/eal_pci.c > index 936f076..5146385 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c > @@ -479,7 +479,7 @@ parse_pci_addr_format(const char *buf, int bufsize, > struct rte_pci_addr *addr) > * list > */ > int > -rte_eal_pci_scan(void) > +pci_scan(void) > { > struct dirent *e; > DIR *dir; > @@ -810,19 +810,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p) > > return ret; > } > - > -/* Init the PCI EAL subsystem */ > -int > -rte_eal_pci_init(void) > -{ > - /* for debug purposes, PCI can be disabled */ > - if (internal_config.no_pci) > - return 0; > - > - if (rte_eal_pci_scan() < 0) { > - RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__); > - return -1; > - } > - > - return 0; > -} > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > index 83721ba..856728e 100644 > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > @@ -44,7 +44,6 @@ DPDK_2.0 { > rte_eal_pci_probe; > rte_eal_pci_probe_one; > rte_eal_pci_register; > - rte_eal_pci_scan; > rte_eal_pci_unregister; > rte_eal_process_type; > rte_eal_remote_launch; > -- - Shreyansh
