Re: [dpdk-dev] [PATCH v2 2/6] eal: Close file descriptor of uio configuration

[Tetsuya Mukawa <mukawa@igel.co.jp>]

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