On Fri, 13 May 2016 09:22:23 +0800 Jianbo Liu <jianbo.liu at linaro.org> wrote:
> On 6 May 2016 at 21:47, Jan Viktorin <viktorin at rehivetech.com> wrote: > > Generalize the PCI-specific pci_unbind_kernel_driver. It is now divided into > > two parts. First, determination of the path and string identification of the > > device to be unbound. Second, the actual unbind operation which is generic. > > > > Signed-off-by: Jan Viktorin <viktorin at rehivetech.com> > > --- > > lib/librte_eal/common/eal_private.h | 13 +++++++++++++ > > lib/librte_eal/linuxapp/eal/eal.c | 26 ++++++++++++++++++++++++++ > > lib/librte_eal/linuxapp/eal/eal_pci.c | 33 > > +++++++++------------------------ > > 3 files changed, 48 insertions(+), 24 deletions(-) > > > > diff --git a/lib/librte_eal/common/eal_private.h > > b/lib/librte_eal/common/eal_private.h > > index 81816a6..3fb8353 100644 > > --- a/lib/librte_eal/common/eal_private.h > > +++ b/lib/librte_eal/common/eal_private.h > > @@ -289,6 +289,19 @@ int rte_eal_alarm_init(void); > > int rte_eal_check_module(const char *module_name); > > > > /** > > + * Unbind kernel driver bound to the device specified by the given devpath, > > + * and its string identification. > > + * > > + * @param devpath path to the device directory ("/sys/.../devices/<name>") > > + * @param devid identification of the device (<name>) > > + * > > + * @return > > + * -1 unbind has failed > > + * 0 module has been unbound > > + */ > > +int rte_eal_unbind_kernel_driver(const char *devpath, const char *devid); > > + > > +/** > > * Get cpu core_id. > > * > > * This function is private to the EAL. > > diff --git a/lib/librte_eal/linuxapp/eal/eal.c > > b/lib/librte_eal/linuxapp/eal/eal.c > > index e8fce6b..844f958 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal.c > > +++ b/lib/librte_eal/linuxapp/eal/eal.c > > @@ -949,3 +949,29 @@ rte_eal_check_module(const char *module_name) > > /* Module has been found */ > > return 1; > > } > > + > > +int > > +rte_eal_unbind_kernel_driver(const char *devpath, const char *devid) > > +{ > > + char filename[PATH_MAX]; > > + FILE *f; > > + > > + snprintf(filename, sizeof(filename), > > + "%s/driver/unbind", devpath); > > + > > + f = fopen(filename, "w"); > > + if (f == NULL) /* device was not bound */ > > + return 0; > > + > > + if (fwrite(devid, strlen(devid), 1, f) == 0) { > > + RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__, > > + filename); > > + goto error; > > + } > > + > > + fclose(f); > > + return 0; > > +error: > > + fclose(f); > > + return -1; > > +} > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c > > b/lib/librte_eal/linuxapp/eal/eal_pci.c > > index fd7e34f..312cb14 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c > > @@ -59,38 +59,23 @@ int > > pci_unbind_kernel_driver(struct rte_pci_device *dev) > > { > > int n; > > - FILE *f; > > - char filename[PATH_MAX]; > > - char buf[BUFSIZ]; > > + char devpath[PATH_MAX]; > > + char devid[BUFSIZ]; > > struct rte_pci_addr *loc = &dev->addr; > > > > - /* open /sys/bus/pci/devices/AAAA:BB:CC.D/driver */ > > - snprintf(filename, sizeof(filename), > > - SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver/unbind", > > + /* devpath /sys/bus/pci/devices/AAAA:BB:CC.D */ > > + snprintf(devpath, sizeof(devpath), > > + SYSFS_PCI_DEVICES "/" PCI_PRI_FMT, > > loc->domain, loc->bus, loc->devid, loc->function); > > > > - f = fopen(filename, "w"); > > - if (f == NULL) /* device was not bound */ > > - return 0; > > - > > - n = snprintf(buf, sizeof(buf), PCI_PRI_FMT "\n", > > + n = snprintf(devid, sizeof(devid), PCI_PRI_FMT "\n", > > loc->domain, loc->bus, loc->devid, loc->function); > > - if ((n < 0) || (n >= (int)sizeof(buf))) { > > + if ((n < 0) || (n >= (int)sizeof(devid))) { > > Is it better to move "(n >= (int)sizeof(devid))" before snprintf and > it has different reason from "n < 0"? I don't understant this comment. I cannot move the check for _n_ before the snprintf as it is its return value... Can you provide an example of your idea? Do you mean to split the condition to if (n < 0) and else if (n >= ...)? > > > RTE_LOG(ERR, EAL, "%s(): snprintf failed\n", __func__); > > - goto error; > > - } > > - if (fwrite(buf, n, 1, f) == 0) { > > - RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__, > > - filename); > > - goto error; > > + return -1; > > } > > > > - fclose(f); > > - return 0; > > - > > -error: > > - fclose(f); > > - return -1; > > + return rte_eal_unbind_kernel_driver(devpath, devid); > > } > > > > static int > > -- > > 2.8.0 > > -- Jan Viktorin E-mail: Viktorin at RehiveTech.com System Architect Web: www.RehiveTech.com RehiveTech Brno, Czech Republic