From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wes1-so2.wedos.net (wes1-so2.wedos.net [46.28.106.16]) by dpdk.org (Postfix) with ESMTP id C07865A97 for ; Tue, 17 May 2016 20:16:09 +0200 (CEST) Received: from pcviktorin.fit.vutbr.cz (pcviktorin.fit.vutbr.cz [147.229.13.147]) by wes1-so2.wedos.net (Postfix) with ESMTPSA id 3r8QWF3vWPz75Y; Tue, 17 May 2016 20:16:09 +0200 (CEST) Date: Tue, 17 May 2016 20:14:24 +0200 From: Jan Viktorin To: Jianbo Liu Cc: dev@dpdk.org, David Marchand , Thomas Monjalon , Bruce Richardson , Declan Doherty , Jerin Jacob , Keith Wiles , Stephen Hemminger Message-ID: <20160517201424.4430b7f0@pcviktorin.fit.vutbr.cz> In-Reply-To: References: <1451682326-5834-1-git-send-email-viktorin@rehivetech.com> <1462542490-15556-1-git-send-email-viktorin@rehivetech.com> <1462542490-15556-4-git-send-email-viktorin@rehivetech.com> Organization: RehiveTech MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 03/28] eal/linux: extract function rte_eal_unbind_kernel_driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 May 2016 18:16:09 -0000 On Fri, 13 May 2016 09:22:23 +0800 Jianbo Liu wrote: > On 6 May 2016 at 21:47, Jan Viktorin 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 > > --- > > 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/") > > + * @param devid identification of the device () > > + * > > + * @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@RehiveTech.com System Architect Web: www.RehiveTech.com RehiveTech Brno, Czech Republic