Re: [dpdk-dev] [PATCH v4 0/5] Clean up pci uio implementations

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

Hi Bernard,


I've updated the series, because I've found the issue in BSD code.
Also, I have rebased the series for latest master branch.

Could you please read my below comments?


On 2015/05/19 14:54, Tetsuya Mukawa wrote:
> This patch set cleans up pci uio implementation. These clean up are
> for consolidating pci uio implementation of linuxapp and bsdapp, and
> moving consolidated functions in eal common.
> Because of above, this patch set tries to implement linuxapp and bsdapp
> almost same.
> Actual consolidations will be done later patch set.
>
> PATCH v4 changes:
>  - Rebase to latest master branch.
>  - Fix bug in pci_uio_map_resource() of BSD code. 'maps[i].path' shouldn't be freed.
>      Fixed in below patch:
>      [PATCH 3/5] eal: Fix memory leaks and needless increment of pci_map_addr

pci_uio_map_resource() {
         ....snip....
-       for (i = 0; i < map_idx; i++)
-               rte_free(maps[i].path);
         ....snip....
}

I removed above code from BSD, because 'maps[i].path' will not be
allocated in BSD code.



>  - 'path' member of 'struct mapped_pci_resource' should not be removed because it will be used in BSD code.
>      Fixed in below patch:
>      [PATCH 5/5] eal: Fix uio mapping differences between linuxapp and bsdapp

struct mapped_pci_resource {
        .....snip.....
        char path[PATH_MAX];
        .....snip.....
 };

I left 'path' member like above. It will be used by BSD code even after
BSD support port hotplug function.

Regards,
Tetsuya

>
> PATCH v3 changes:
>  - Squash patches related with pci_map_resource().
>  - Free maps[].path to easy to understand.
>    (Thanks to Iremonger, Bernard)
>  - Close fds opened in this function.
>  - Remove unused path variable from mapped_pci_resource structure.
>
> PATCH v2 changes:
>  - Move 'if-condition' to later patch series.
>  - Fix memory leaks of path.
>  - Fix typos.
>    (Thanks to David Marchand)
>  - Fix commit title and body.
>  - Fix pci_map_resource() to handle MAP_FAILED.
>    (Thanks to Iremonger, Bernard)
>
> Changes:
>  - This patch set is derived from below.
>    "[PATCH v2] eal: Port Hotplug support for BSD"
>  - Set cfg_fd as -1, when cfg_fd is closed.
>    (Thanks to Iremonger, Bernard)
>  - Remove needless coding style fixings.
>  - Fix coding style of if-else condition.
>    (Thanks to Richardson, Bruce)
>
>
> Tetsuya Mukawa (5):
>   eal: Fix coding style of eal_pci.c and eal_pci_uio.c
>   eal: Close file descriptor of uio configuration
>   eal: Fix memory leaks and needless increment of pci_map_addr
>   eal/bsdapp: Change names of pci related data structure
>   eal: Fix uio mapping differences between linuxapp and bsdapp
>
>  lib/librte_eal/bsdapp/eal/eal_pci.c       | 164 ++++++++++++++++++------------
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c |  88 +++++++++-------
>  2 files changed, 153 insertions(+), 99 deletions(-)
>