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
