From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f49.google.com (mail-wg0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id 236F36A95 for ; Thu, 16 Apr 2015 09:50:37 +0200 (CEST) Received: by wgso17 with SMTP id o17so71593651wgs.1 for ; Thu, 16 Apr 2015 00:50:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:references:in-reply-to:subject:date:message-id:mime-version :content-type:content-transfer-encoding:thread-index :content-language; bh=ORFH5l8h+WhILryGJsCKY/MOWWaSjNiJGMUSJ+lgPqY=; b=ogJUnsiyvgiDmFYSKkV8F6MBnXD2cGNuYTniBcdWcJDJamJrKpOktvbOqV/elGKsWu cbXcvFYsSCjkD1Ox0+Rk/iqlW+0o5ULBbsyOR2jk2yCnkpQr/FYq/c+VqO0VXYN8cycJ d9sbnsOQPeZ2Qb1TWjqom2iDegJXQCxo4RyZleZZ7MJIlzXRhEeZvtE8RZtv0tFmteJ2 rR08qjPH/aZkSsDEWl83EwpXEnaqUL/abDUj+fCBw87ZiPtOLaMK0sJeO+wUGpuBtU9t aUf8P5akfy1wqoIQJM9CnElthxArQ5IpPC/pdVIoM3/Ht+7dL/UvD1N5SSqgcvb2vz45 mwZw== X-Received: by 10.180.92.65 with SMTP id ck1mr2637130wib.78.1429170636990; Thu, 16 Apr 2015 00:50:36 -0700 (PDT) Received: from laptop1 (84.95.210.61.forward.012.net.il. [84.95.210.61]) by mx.google.com with ESMTPSA id df1sm24836499wib.12.2015.04.16.00.50.35 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 16 Apr 2015 00:50:36 -0700 (PDT) From: "Raz Amir" To: "'Ouyang, Changchun'" , "'Ananyev, Konstantin'" , "'Thomas Monjalon'" , References: <1428450303-97954-1-git-send-email-razamir22@gmail.com> <1429028594-12323-1-git-send-email-razamir22@gmail.com> <2601191342CEEE43887BDE71AB97725821415A1D@irsmsx105.ger.corp.intel.com> In-Reply-To: Date: Thu, 16 Apr 2015 10:50:33 +0300 Message-ID: <169001d0781a$05ed3960$11c7ac20$@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 15.0 Thread-Index: AQKh04vm1iLk0SVFuqC8rqR6FNNrlgMusfo9AUVGa0wCEN8KG5t4ZMfQ Content-Language: en-gb Subject: Re: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD 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: Thu, 16 Apr 2015 07:50:37 -0000 Hi, >>From both running and reading the code, the rte_virtio_pmd_init is called only once from: rte_eal_init -> rte_eal_dev_init. But, the uninit won't be called, since uninit it called only for PMD_VDEV driver types, while virtio is PMD_PDEV. Based on that, I am going to submit the original patch again, that fd won't be closed, and without handling the close at uninit as it isn't called and since fd will be closed anyway when the process exits. Thanks, Raz. -----Original Message----- From: Ouyang, Changchun [mailto:changchun.ouyang@intel.com] Sent: 16 April 2015 06:31 To: Ananyev, Konstantin; Raz Amir; dev@dpdk.org Cc: Ouyang, Changchun Subject: RE: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, > Konstantin > Sent: Wednesday, April 15, 2015 6:22 AM > To: Raz Amir; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5] Restore support for virtio on > FreeBSD > > Hi, > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Raz Amir > > Sent: Tuesday, April 14, 2015 5:23 PM > > To: dev@dpdk.org > > Cc: Raz Amir > > Subject: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD > > > > Fixes: 8a312224bcde ("eal/bsd: fix fd leak") > > > > Closing /dev/io fd causes SIGBUS in inb/outb instructions as the > > process loses the IOPL privileges once the fd is closed: > > (gdb) bt > > 0 0x0000000000492f2c in outb (port=49170, data=0 '\000') > > at /usr/include/machine/cpufunc.h:244 > > 1 0x0000000000492f7a in outb_p (data=0 '\000', port=49170) > > at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211 > > 2 0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0 > '\000') > > at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130 > > 3 0x00000000004931fe in vtpci_reset (hw=0x80331f380) > > at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108 > > 4 0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80 > ) > > at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150 > > 5 0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0 > , > > pci_dev=0x802417560) at > > /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326 > > 6 0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0 > , > > dev=0x802417560) at > > /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487 > > 7 0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560) > > at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116 > > 8 0x0000000000475bb9 in rte_eal_pci_probe () > > at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246 > > 9 0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0) > > at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554 > > 10 0x0000000000404544 in main () > > > > Signed-off-by: Raz Amir > > --- > > lib/librte_eal/bsdapp/eal/eal.c | 19 ++++++++++++++----- > > lib/librte_eal/common/include/rte_eal.h | 10 ++++++++++ > > lib/librte_eal/linuxapp/eal/eal.c | 5 +++++ > > lib/librte_pmd_virtio/virtio_ethdev.c | 9 +++++++++ > > 4 files changed, 38 insertions(+), 5 deletions(-) > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c > > b/lib/librte_eal/bsdapp/eal/eal.c index 871d5f4..687dd83 100644 > > --- a/lib/librte_eal/bsdapp/eal/eal.c > > +++ b/lib/librte_eal/bsdapp/eal/eal.c > > @@ -112,6 +112,9 @@ struct internal_config internal_config; > > /* used by rte_rdtsc() */ > > int rte_cycles_vmware_tsc_map; > > > > +/* fd to keep open for iopl */ > > +static int iopl_fd = -1; > > + > > /* Return a pointer to the configuration structure */ struct > > rte_config * > > rte_eal_get_configuration(void) > > @@ -421,15 +424,21 @@ int rte_eal_has_hugepages(void) int > > rte_eal_iopl_init(void) > > { > > - int fd; > > - > > - fd = open("/dev/io", O_RDWR); > > - if (fd < 0) > > + iopl_fd = open("/dev/io", O_RDWR); > > + if (iopl_fd < 0) > > return -1; > > - close(fd); > > + /* keep fd open for iopl */ > > return 0; > > } > > > > +void > > +rte_eal_iopl_uninit(void) > > +{ > > + if (iopl_fd != -1) > > + close(iopl_fd); > > + iopl_fd = -1; > > +} > > Did I get it right: that function would be invoked for at dev_detach()? > And after we invoked it, we still we can have other multiple virtio > devices attached and active? > If so, then I suppose you'll hit the same problem again. > Konstantin > Yes, need verify this issue, If it is true, may use reference counter to resolve it? Thanks Changchun > > + > > /* Launch threads, called at application init(). */ int > > rte_eal_init(int argc, char **argv) diff --git > > a/lib/librte_eal/common/include/rte_eal.h > > b/lib/librte_eal/common/include/rte_eal.h > > index 1385a73..9151e08 100644 > > --- a/lib/librte_eal/common/include/rte_eal.h > > +++ b/lib/librte_eal/common/include/rte_eal.h > > @@ -127,6 +127,16 @@ enum rte_proc_type_t > rte_eal_process_type(void); > > int rte_eal_iopl_init(void); > > > > /** > > + * Release iopl priviledge - currently relevant only for FreeBSD. > > + * > > + * This function should be called by pmds which need access to ioports. > > + > > + * @return > > + * void > > + */ > > +void rte_eal_iopl_uninit(void); > > + > > +/** > > * Initialize the Environment Abstraction Layer (EAL). > > * > > * This function is to be executed on the MASTER lcore only, as > > soon diff --git a/lib/librte_eal/linuxapp/eal/eal.c > > b/lib/librte_eal/linuxapp/eal/eal.c > > index bd770cf..687cebf 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal.c > > +++ b/lib/librte_eal/linuxapp/eal/eal.c > > @@ -695,6 +695,11 @@ rte_eal_iopl_init(void) #endif } > > > > +void > > +rte_eal_iopl_uninit(void) > > +{ > > +} > > + > > /* Launch threads, called at application init(). */ int > > rte_eal_init(int argc, char **argv) diff --git > > a/lib/librte_pmd_virtio/virtio_ethdev.c > > b/lib/librte_pmd_virtio/virtio_ethdev.c > > index 7b83d9b..5be5c27 100644 > > --- a/lib/librte_pmd_virtio/virtio_ethdev.c > > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c > > @@ -1265,6 +1265,14 @@ rte_virtio_pmd_init(const char *name > __rte_unused, > > return 0; > > } > > > > +static int > > +rte_virtio_pmd_uninit(const char *name) { > > + (void)name; > > + rte_eal_iopl_uninit(); > > + return 0; > > +} > > + > > /* > > * Only 1 queue is supported, no queue release related operation > > */ > > @@ -1499,6 +1507,7 @@ __rte_unused uint8_t is_rx) static struct > > rte_driver rte_virtio_driver = { > > .type = PMD_PDEV, > > .init = rte_virtio_pmd_init, > > + .uninit = rte_virtio_pmd_uninit, > > }; > > > > PMD_REGISTER_DRIVER(rte_virtio_driver); > > -- > > 2.1.2