Message ID | 1427170717-13879-3-git-send-email-mukawa@igel.co.jp (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 960A69AB7; Tue, 24 Mar 2015 05:18:58 +0100 (CET) Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com [209.85.220.46]) by dpdk.org (Postfix) with ESMTP id 574499AB0 for <dev@dpdk.org>; Tue, 24 Mar 2015 05:18:56 +0100 (CET) Received: by padcy3 with SMTP id cy3so212815597pad.3 for <dev@dpdk.org>; Mon, 23 Mar 2015 21:18:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=h5Iv+EizamM1qseoZrG0i8pO12E9Q5eXyFW1nRbB0VU=; b=Fz6cSvGXFrsJL4/PZXR6JD/bCD4vimFEuDwlWE+ezUwc/jSntL55HxOBl/xUSWNimE o7hkHrR7/V0LbSmOc32wt4qk9WejJxh3SZShiRdks2g6WBqmMT6RfwsYgI+kH8O7hnZW yt7XofWV8SLRevUjhUhNQX3rjaQBhNTye4RbNTIDwJapOmN9EXwmsgyIRI0S5sRzRS59 CMkpigNWc5hMSRk0PSo1he0lISE+tSn4HHTGY7fKMHSc/JzyypDwOVRtIYgFL9qMWcKy Zmv3JBjVrsoz+NcFtQ9BRKfygcZ4zxnITUga1T6yDZK7I1k+DCLzQlg5qcsZCIDdE4yz NogA== X-Gm-Message-State: ALoCoQkjojqGZgGa8CeQPyvN+fGLpwVi5D87Ji8Wd59XsBltgVzjxeoseDa+/1NOpbsx2aAuzGQK X-Received: by 10.69.12.199 with SMTP id es7mr4212706pbd.138.1427170735788; Mon, 23 Mar 2015 21:18:55 -0700 (PDT) Received: from localhost.localdomain (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id jx5sm2482078pbc.85.2015.03.23.21.18.53 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 23 Mar 2015 21:18:55 -0700 (PDT) From: Tetsuya Mukawa <mukawa@igel.co.jp> To: dev@dpdk.org Date: Tue, 24 Mar 2015 13:18:33 +0900 Message-Id: <1427170717-13879-3-git-send-email-mukawa@igel.co.jp> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1427170717-13879-1-git-send-email-mukawa@igel.co.jp> References: <1426584645-28828-7-git-send-email-mukawa@igel.co.jp> <1427170717-13879-1-git-send-email-mukawa@igel.co.jp> Subject: [dpdk-dev] [PATCH v2 2/6] eal: Close file descriptor of uio configuration X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Tetsuya Mukawa
March 24, 2015, 4:18 a.m. UTC
When pci_uio_unmap_resource() is called, a file descriptor that is used
for uio configuration should be closed.
Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Comments
On Tue, 24 Mar 2015 13:18:33 +0900 Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > When pci_uio_unmap_resource() is called, a file descriptor that is used > for uio configuration should be closed. > > Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> > --- > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > index 9cdf24f..f0277be 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > @@ -459,8 +459,12 @@ pci_uio_unmap_resource(struct rte_pci_device *dev) > > /* close fd if in primary process */ > close(dev->intr_handle.fd); > - > dev->intr_handle.fd = -1; > + > + /* close cfg_fd if in primary process */ > + close(dev->intr_handle.uio_cfg_fd); > + dev->intr_handle.uio_cfg_fd = -1; > + > dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; > } > #endif /* RTE_LIBRTE_EAL_HOTPLUG */ For the Qlogic/Broadcom driver it needed the config fd handle, and I added generic config space access functions.
On 2015/03/25 3:33, Stephen Hemminger wrote: > On Tue, 24 Mar 2015 13:18:33 +0900 > Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > >> When pci_uio_unmap_resource() is called, a file descriptor that is used >> for uio configuration should be closed. >> >> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> >> --- >> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> index 9cdf24f..f0277be 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> @@ -459,8 +459,12 @@ pci_uio_unmap_resource(struct rte_pci_device *dev) >> >> /* close fd if in primary process */ >> close(dev->intr_handle.fd); >> - >> dev->intr_handle.fd = -1; >> + >> + /* close cfg_fd if in primary process */ >> + close(dev->intr_handle.uio_cfg_fd); >> + dev->intr_handle.uio_cfg_fd = -1; >> + >> dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; >> } >> #endif /* RTE_LIBRTE_EAL_HOTPLUG */ > > For the Qlogic/Broadcom driver it needed the config fd handle, and I added > generic config space access functions. Hi Stephen, Is this the patch you mentioned? http://dpdk.org/dev/patchwork/patch/3024/ Hi David, Bernard, Stephen I guess here are works we will need to do. 1. Add close(dev->config_fd) in Stephen's patch. 2. Write a patch for uio to merge "dev->intr_handle->uio_cfg_fd" and "dev->config_fd". 3. Write a patch for vfio to merge "dev->intr_handle->vfio_cfg_fd" and "dev->config_fd". If we already have these patches, I guess it may be nice to merge above patches first. Do you have a suggestion how to merge patches related with pci config fd? Thanks, Tetsuya
On Wed, 25 Mar 2015 12:17:32 +0900 Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > On 2015/03/25 3:33, Stephen Hemminger wrote: > > On Tue, 24 Mar 2015 13:18:33 +0900 > > Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > > > >> When pci_uio_unmap_resource() is called, a file descriptor that is used > >> for uio configuration should be closed. > >> > >> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> > >> --- > >> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > >> index 9cdf24f..f0277be 100644 > >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > >> @@ -459,8 +459,12 @@ pci_uio_unmap_resource(struct rte_pci_device *dev) > >> > >> /* close fd if in primary process */ > >> close(dev->intr_handle.fd); > >> - > >> dev->intr_handle.fd = -1; > >> + > >> + /* close cfg_fd if in primary process */ > >> + close(dev->intr_handle.uio_cfg_fd); > >> + dev->intr_handle.uio_cfg_fd = -1; > >> + > >> dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; > >> } > >> #endif /* RTE_LIBRTE_EAL_HOTPLUG */ > > > > For the Qlogic/Broadcom driver it needed the config fd handle, and I added > > generic config space access functions. > > Hi Stephen, > > Is this the patch you mentioned? > http://dpdk.org/dev/patchwork/patch/3024/ > > > Hi David, Bernard, Stephen > > I guess here are works we will need to do. > 1. Add close(dev->config_fd) in Stephen's patch. > 2. Write a patch for uio to merge "dev->intr_handle->uio_cfg_fd" and > "dev->config_fd". > 3. Write a patch for vfio to merge "dev->intr_handle->vfio_cfg_fd" and > "dev->config_fd". > > If we already have these patches, I guess it may be nice to merge above > patches first. > Do you have a suggestion how to merge patches related with pci config fd? > > Thanks, > Tetsuya > Yeah, that is the patch. It reopens config fd, it seems to overlap this.
On 2015/03/25 14:07, Stephen Hemminger wrote: > On Wed, 25 Mar 2015 12:17:32 +0900 > Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > >> On 2015/03/25 3:33, Stephen Hemminger wrote: >>> On Tue, 24 Mar 2015 13:18:33 +0900 >>> Tetsuya Mukawa <mukawa@igel.co.jp> wrote: >>> >>>> When pci_uio_unmap_resource() is called, a file descriptor that is used >>>> for uio configuration should be closed. >>>> >>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> >>>> --- >>>> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>>> index 9cdf24f..f0277be 100644 >>>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>>> @@ -459,8 +459,12 @@ pci_uio_unmap_resource(struct rte_pci_device *dev) >>>> >>>> /* close fd if in primary process */ >>>> close(dev->intr_handle.fd); >>>> - >>>> dev->intr_handle.fd = -1; >>>> + >>>> + /* close cfg_fd if in primary process */ >>>> + close(dev->intr_handle.uio_cfg_fd); >>>> + dev->intr_handle.uio_cfg_fd = -1; >>>> + >>>> dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; >>>> } >>>> #endif /* RTE_LIBRTE_EAL_HOTPLUG */ >>> For the Qlogic/Broadcom driver it needed the config fd handle, and I added >>> generic config space access functions. >> Hi Stephen, >> >> Is this the patch you mentioned? >> http://dpdk.org/dev/patchwork/patch/3024/ >> >> >> Hi David, Bernard, Stephen >> >> I guess here are works we will need to do. >> 1. Add close(dev->config_fd) in Stephen's patch. >> 2. Write a patch for uio to merge "dev->intr_handle->uio_cfg_fd" and >> "dev->config_fd". >> 3. Write a patch for vfio to merge "dev->intr_handle->vfio_cfg_fd" and >> "dev->config_fd". >> >> If we already have these patches, I guess it may be nice to merge above >> patches first. >> Do you have a suggestion how to merge patches related with pci config fd? >> >> Thanks, >> Tetsuya >> > Yeah, that is the patch. It reopens config fd, it seems to overlap > this. Hi Stephen, David, Bernard, If you are OK, I will cherry pick below Stephen's patch, then put it on top of my patches. - http://dpdk.org/dev/patchwork/patch/3024/ Also I will write patches to merge following fds. - dev->config_fd - dev->intr_handle->uio_cfg_fd - dev->intr_handle->vfio_cfg_fd Is this direction OK? Stephen, For uio, I guess it will be OK that I just replace pread/pwrite by your APIs. But for vfio, I need to write a function to wrap vfio ioctl. May be rte_eal_pci_ioctl_config()? And replace all vfio ioctls by the function. Is this correct way to adopt your APIs for vfio? Regards, Tetsuya
> -----Original Message----- > From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] > Sent: Thursday, March 26, 2015 4:19 AM > To: Stephen Hemminger > Cc: Iremonger, Bernard; david.marchand@6wind.com; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 2/6] eal: Close file descriptor of uio configuration > > On 2015/03/25 14:07, Stephen Hemminger wrote: > > On Wed, 25 Mar 2015 12:17:32 +0900 > > Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > > > >> On 2015/03/25 3:33, Stephen Hemminger wrote: > >>> On Tue, 24 Mar 2015 13:18:33 +0900 > >>> Tetsuya Mukawa <mukawa@igel.co.jp> wrote: > >>> > >>>> When pci_uio_unmap_resource() is called, a file descriptor that is > >>>> used for uio configuration should be closed. > >>>> > >>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> > >>>> --- > >>>> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 6 +++++- > >>>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > >>>> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > >>>> index 9cdf24f..f0277be 100644 > >>>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > >>>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > >>>> @@ -459,8 +459,12 @@ pci_uio_unmap_resource(struct rte_pci_device > >>>> *dev) > >>>> > >>>> /* close fd if in primary process */ > >>>> close(dev->intr_handle.fd); > >>>> - > >>>> dev->intr_handle.fd = -1; > >>>> + > >>>> + /* close cfg_fd if in primary process */ > >>>> + close(dev->intr_handle.uio_cfg_fd); > >>>> + dev->intr_handle.uio_cfg_fd = -1; > >>>> + > >>>> dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; } #endif /* > >>>> RTE_LIBRTE_EAL_HOTPLUG */ > >>> For the Qlogic/Broadcom driver it needed the config fd handle, and I > >>> added generic config space access functions. > >> Hi Stephen, > >> > >> Is this the patch you mentioned? > >> http://dpdk.org/dev/patchwork/patch/3024/ > >> > >> > >> Hi David, Bernard, Stephen > >> > >> I guess here are works we will need to do. > >> 1. Add close(dev->config_fd) in Stephen's patch. > >> 2. Write a patch for uio to merge "dev->intr_handle->uio_cfg_fd" and > >> "dev->config_fd". > >> 3. Write a patch for vfio to merge "dev->intr_handle->vfio_cfg_fd" > >> and "dev->config_fd". > >> > >> If we already have these patches, I guess it may be nice to merge > >> above patches first. > >> Do you have a suggestion how to merge patches related with pci config fd? > >> > >> Thanks, > >> Tetsuya > >> > > Yeah, that is the patch. It reopens config fd, it seems to overlap > > this. > > Hi Stephen, David, Bernard, > > If you are OK, I will cherry pick below Stephen's patch, then put it on top of my patches. > - http://dpdk.org/dev/patchwork/patch/3024/ > > Also I will write patches to merge following fds. > - dev->config_fd > - dev->intr_handle->uio_cfg_fd > - dev->intr_handle->vfio_cfg_fd > > Is this direction OK? > > > Stephen, > > For uio, I guess it will be OK that I just replace pread/pwrite by your APIs. > But for vfio, I need to write a function to wrap vfio ioctl. > May be rte_eal_pci_ioctl_config()? > And replace all vfio ioctls by the function. > Is this correct way to adopt your APIs for vfio? > > Regards, > Tetsuya Hi Tetsuya, It would be better not to broaden the scope of the BSD cleanup patch set at this point. It would be better to have a separate patch set to apply Stephen's changes to the BSD code base. Also, discussion is still ongoing on Stephen's changes so it might be better to wait until the discussion is completed. Regards, Bernard.
On 2015/03/26 19:03, Iremonger, Bernard wrote: > >> -----Original Message----- >> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] >> Sent: Thursday, March 26, 2015 4:19 AM >> To: Stephen Hemminger >> Cc: Iremonger, Bernard; david.marchand@6wind.com; dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v2 2/6] eal: Close file descriptor of uio configuration >> >> On 2015/03/25 14:07, Stephen Hemminger wrote: >>> On Wed, 25 Mar 2015 12:17:32 +0900 >>> Tetsuya Mukawa <mukawa@igel.co.jp> wrote: >>> >>>> On 2015/03/25 3:33, Stephen Hemminger wrote: >>>>> On Tue, 24 Mar 2015 13:18:33 +0900 >>>>> Tetsuya Mukawa <mukawa@igel.co.jp> wrote: >>>>> >>>>>> When pci_uio_unmap_resource() is called, a file descriptor that is >>>>>> used for uio configuration should be closed. >>>>>> >>>>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp> >>>>>> --- >>>>>> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 6 +++++- >>>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>>>>> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>>>>> index 9cdf24f..f0277be 100644 >>>>>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>>>>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >>>>>> @@ -459,8 +459,12 @@ pci_uio_unmap_resource(struct rte_pci_device >>>>>> *dev) >>>>>> >>>>>> /* close fd if in primary process */ >>>>>> close(dev->intr_handle.fd); >>>>>> - >>>>>> dev->intr_handle.fd = -1; >>>>>> + >>>>>> + /* close cfg_fd if in primary process */ >>>>>> + close(dev->intr_handle.uio_cfg_fd); >>>>>> + dev->intr_handle.uio_cfg_fd = -1; >>>>>> + >>>>>> dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; } #endif /* >>>>>> RTE_LIBRTE_EAL_HOTPLUG */ >>>>> For the Qlogic/Broadcom driver it needed the config fd handle, and I >>>>> added generic config space access functions. >>>> Hi Stephen, >>>> >>>> Is this the patch you mentioned? >>>> http://dpdk.org/dev/patchwork/patch/3024/ >>>> >>>> >>>> Hi David, Bernard, Stephen >>>> >>>> I guess here are works we will need to do. >>>> 1. Add close(dev->config_fd) in Stephen's patch. >>>> 2. Write a patch for uio to merge "dev->intr_handle->uio_cfg_fd" and >>>> "dev->config_fd". >>>> 3. Write a patch for vfio to merge "dev->intr_handle->vfio_cfg_fd" >>>> and "dev->config_fd". >>>> >>>> If we already have these patches, I guess it may be nice to merge >>>> above patches first. >>>> Do you have a suggestion how to merge patches related with pci config fd? >>>> >>>> Thanks, >>>> Tetsuya >>>> >>> Yeah, that is the patch. It reopens config fd, it seems to overlap >>> this. >> Hi Stephen, David, Bernard, >> >> If you are OK, I will cherry pick below Stephen's patch, then put it on top of my patches. >> - http://dpdk.org/dev/patchwork/patch/3024/ >> >> Also I will write patches to merge following fds. >> - dev->config_fd >> - dev->intr_handle->uio_cfg_fd >> - dev->intr_handle->vfio_cfg_fd >> >> Is this direction OK? >> >> >> Stephen, >> >> For uio, I guess it will be OK that I just replace pread/pwrite by your APIs. >> But for vfio, I need to write a function to wrap vfio ioctl. >> May be rte_eal_pci_ioctl_config()? >> And replace all vfio ioctls by the function. >> Is this correct way to adopt your APIs for vfio? >> >> Regards, >> Tetsuya > Hi Tetsuya, > > It would be better not to broaden the scope of the BSD cleanup patch set at this point. > It would be better to have a separate patch set to apply Stephen's changes to the BSD code base. > Also, discussion is still ongoing on Stephen's changes so it might be better to wait until the discussion is completed. Hi Bernard, Thanks for comments. I will focus on BSD thing. Regards, Tetsuya > Regards, > > Bernard. > > >
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c index 9cdf24f..f0277be 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c @@ -459,8 +459,12 @@ pci_uio_unmap_resource(struct rte_pci_device *dev) /* close fd if in primary process */ close(dev->intr_handle.fd); - dev->intr_handle.fd = -1; + + /* close cfg_fd if in primary process */ + close(dev->intr_handle.uio_cfg_fd); + dev->intr_handle.uio_cfg_fd = -1; + dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; } #endif /* RTE_LIBRTE_EAL_HOTPLUG */