DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eal: Fix resource leak of pci_uio_map_secondary()
@ 2016-06-15  3:26   ` Tetsuya Mukawa
  2016-06-15 14:45     ` David Marchand
  2016-06-16  2:33     ` [dpdk-dev] [PATCH v2] eal: Fix resource leak while secondary process maps pci devices Tetsuya Mukawa
  0 siblings, 2 replies; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-15  3:26 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, john.mcnamara, michalx.k.jastrzebski, Tetsuya Mukawa

If pci_map_resource() succeeds but mapped address is different from an
address primary process mapped, this should be error.
Then the address secondary process mapped should be freed.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/common/eal_common_pci_uio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
index f062e81..e718643 100644
--- a/lib/librte_eal/common/eal_common_pci_uio.c
+++ b/lib/librte_eal/common/eal_common_pci_uio.c
@@ -85,6 +85,9 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 					"Cannot mmap device resource file %s to address: %p\n",
 					uio_res->maps[i].path,
 					uio_res->maps[i].addr);
+				if (mapaddr != MAP_FAILED)
+					pci_unmap_resource(mapaddr,
+						(size_t)uio_res->maps[i].size);
 				return -1;
 			}
 		}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH] eal: Fix wrong error checking of rte_eal_parse_devargs_str()
@ 2016-06-15  3:26 Tetsuya Mukawa
  2016-06-15 14:48 ` David Marchand
  2016-06-16  2:33 ` [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments Tetsuya Mukawa
  0 siblings, 2 replies; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-15  3:26 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, john.mcnamara, michalx.k.jastrzebski, Tetsuya Mukawa

Currently, a return value of strdup() isn't checked correctly.
The patch fixes it.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/common/eal_common_devargs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 2bfe54a..e403717 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -58,7 +58,7 @@ rte_eal_parse_devargs_str(const char *devargs_str,
 		return -1;
 
 	*drvname = strdup(devargs_str);
-	if (drvname == NULL)
+	if (*drvname == NULL)
 		return -1;
 
 	/* set the first ',' to '\0' to split name and arguments */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH] eal: Fix wrong resource release of pci_uio_unmap()
@ 2016-06-15  3:27   ` Tetsuya Mukawa
  2016-06-15 15:01     ` David Marchand
  2016-06-16  2:33     ` [dpdk-dev] [PATCH v2] eal: Fix wrong resource release while unmapping pci devices Tetsuya Mukawa
  0 siblings, 2 replies; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-15  3:27 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, john.mcnamara, michalx.k.jastrzebski, Tetsuya Mukawa

The 'path' member of mapped_pci_resource structure is allocated by
primary process, but currenctly it will be freed by both primary
and secondary process.
The patch fixes to be freed by only primary process.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/common/eal_common_pci_uio.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
index e718643..b2c68f3 100644
--- a/lib/librte_eal/common/eal_common_pci_uio.c
+++ b/lib/librte_eal/common/eal_common_pci_uio.c
@@ -162,7 +162,9 @@ pci_uio_unmap(struct mapped_pci_resource *uio_res)
 	for (i = 0; i != uio_res->nb_maps; i++) {
 		pci_unmap_resource(uio_res->maps[i].addr,
 				(size_t)uio_res->maps[i].size);
-		rte_free(uio_res->maps[i].path);
+
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+			rte_free(uio_res->maps[i].path);
 	}
 }
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: Fix resource leak of pci_uio_map_secondary()
  2016-06-15  3:26   ` [dpdk-dev] [PATCH] eal: Fix resource leak of pci_uio_map_secondary() Tetsuya Mukawa
@ 2016-06-15 14:45     ` David Marchand
  2016-06-16  2:03       ` Tetsuya Mukawa
  2016-06-16  2:33     ` [dpdk-dev] [PATCH v2] eal: Fix resource leak while secondary process maps pci devices Tetsuya Mukawa
  1 sibling, 1 reply; 18+ messages in thread
From: David Marchand @ 2016-06-15 14:45 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev, Mcnamara, John, michalx.k.jastrzebski

Hello Tetsuya,

On Wed, Jun 15, 2016 at 5:26 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> If pci_map_resource() succeeds but mapped address is different from an
> address primary process mapped, this should be error.
> Then the address secondary process mapped should be freed.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

The title should not contain the function name.


> ---
>  lib/librte_eal/common/eal_common_pci_uio.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
> index f062e81..e718643 100644
> --- a/lib/librte_eal/common/eal_common_pci_uio.c
> +++ b/lib/librte_eal/common/eal_common_pci_uio.c
> @@ -85,6 +85,9 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>                                         "Cannot mmap device resource file %s to address: %p\n",
>                                         uio_res->maps[i].path,
>                                         uio_res->maps[i].addr);
> +                               if (mapaddr != MAP_FAILED)
> +                                       pci_unmap_resource(mapaddr,
> +                                               (size_t)uio_res->maps[i].size);
>                                 return -1;
>                         }
>                 }

What of the previous mappings that might have succeeded earlier in this loop ?


-- 
David Marchand

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: Fix wrong error checking of rte_eal_parse_devargs_str()
  2016-06-15  3:26 [dpdk-dev] [PATCH] eal: Fix wrong error checking of rte_eal_parse_devargs_str() Tetsuya Mukawa
@ 2016-06-15 14:48 ` David Marchand
  2016-06-16  2:03   ` Tetsuya Mukawa
  2016-06-16  2:33 ` [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments Tetsuya Mukawa
  1 sibling, 1 reply; 18+ messages in thread
From: David Marchand @ 2016-06-15 14:48 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev, Mcnamara, John, michalx.k.jastrzebski

On Wed, Jun 15, 2016 at 5:26 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> Currently, a return value of strdup() isn't checked correctly.
> The patch fixes it.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Idem, the title does not need the function name to describe what is going wrong.

This might deserve a fixline, I traced this to 0fe11ec592b2 ("eal: add
vdev init and uninit").

> ---
>  lib/librte_eal/common/eal_common_devargs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index 2bfe54a..e403717 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -58,7 +58,7 @@ rte_eal_parse_devargs_str(const char *devargs_str,
>                 return -1;
>
>         *drvname = strdup(devargs_str);
> -       if (drvname == NULL)
> +       if (*drvname == NULL)
>                 return -1;
>
>         /* set the first ',' to '\0' to split name and arguments */
> --
> 2.7.4
>

-- 
David Marchand

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: Fix wrong resource release of pci_uio_unmap()
  2016-06-15  3:27   ` [dpdk-dev] [PATCH] eal: Fix wrong resource release of pci_uio_unmap() Tetsuya Mukawa
@ 2016-06-15 15:01     ` David Marchand
  2016-06-16  2:33     ` [dpdk-dev] [PATCH v2] eal: Fix wrong resource release while unmapping pci devices Tetsuya Mukawa
  1 sibling, 0 replies; 18+ messages in thread
From: David Marchand @ 2016-06-15 15:01 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev, Mcnamara, John, michalx.k.jastrzebski

On Wed, Jun 15, 2016 at 5:27 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> The 'path' member of mapped_pci_resource structure is allocated by
> primary process, but currenctly it will be freed by both primary
> and secondary process.
> The patch fixes to be freed by only primary process.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Function name in title.

> ---
>  lib/librte_eal/common/eal_common_pci_uio.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
> index e718643..b2c68f3 100644
> --- a/lib/librte_eal/common/eal_common_pci_uio.c
> +++ b/lib/librte_eal/common/eal_common_pci_uio.c
> @@ -162,7 +162,9 @@ pci_uio_unmap(struct mapped_pci_resource *uio_res)
>         for (i = 0; i != uio_res->nb_maps; i++) {
>                 pci_unmap_resource(uio_res->maps[i].addr,
>                                 (size_t)uio_res->maps[i].size);
> -               rte_free(uio_res->maps[i].path);
> +
> +               if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +                       rte_free(uio_res->maps[i].path);
>         }
>  }
>

The rest looks good to me.

-- 
David Marchand

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: Fix resource leak of pci_uio_map_secondary()
  2016-06-15 14:45     ` David Marchand
@ 2016-06-16  2:03       ` Tetsuya Mukawa
  0 siblings, 0 replies; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-16  2:03 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Mcnamara, John, michalx.k.jastrzebski

On 2016/06/15 23:45, David Marchand wrote:
> Hello Tetsuya,
> 
> On Wed, Jun 15, 2016 at 5:26 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
>> If pci_map_resource() succeeds but mapped address is different from an
>> address primary process mapped, this should be error.
>> Then the address secondary process mapped should be freed.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> 
> The title should not contain the function name.

Hi David,

Thanks, I will change the title of all patches.

> 
> 
>> ---
>>  lib/librte_eal/common/eal_common_pci_uio.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
>> index f062e81..e718643 100644
>> --- a/lib/librte_eal/common/eal_common_pci_uio.c
>> +++ b/lib/librte_eal/common/eal_common_pci_uio.c
>> @@ -85,6 +85,9 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>>                                         "Cannot mmap device resource file %s to address: %p\n",
>>                                         uio_res->maps[i].path,
>>                                         uio_res->maps[i].addr);
>> +                               if (mapaddr != MAP_FAILED)
>> +                                       pci_unmap_resource(mapaddr,
>> +                                               (size_t)uio_res->maps[i].size);
>>                                 return -1;
>>                         }
>>                 }
> 
> What of the previous mappings that might have succeeded earlier in this loop ?

I will unmap all addresses mapped before.

Thanks,
Tetsuya

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: Fix wrong error checking of rte_eal_parse_devargs_str()
  2016-06-15 14:48 ` David Marchand
@ 2016-06-16  2:03   ` Tetsuya Mukawa
  0 siblings, 0 replies; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-16  2:03 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Mcnamara, John, michalx.k.jastrzebski

On 2016/06/15 23:48, David Marchand wrote:
> On Wed, Jun 15, 2016 at 5:26 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
>> Currently, a return value of strdup() isn't checked correctly.
>> The patch fixes it.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> 
> Idem, the title does not need the function name to describe what is going wrong.
> 
> This might deserve a fixline, I traced this to 0fe11ec592b2 ("eal: add
> vdev init and uninit").

Will add it.

Thanks,
Tetsuya

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments
  2016-06-15  3:26 [dpdk-dev] [PATCH] eal: Fix wrong error checking of rte_eal_parse_devargs_str() Tetsuya Mukawa
  2016-06-15 14:48 ` David Marchand
@ 2016-06-16  2:33 ` Tetsuya Mukawa
  2016-06-15  3:26   ` [dpdk-dev] [PATCH] eal: Fix resource leak of pci_uio_map_secondary() Tetsuya Mukawa
                     ` (2 more replies)
  1 sibling, 3 replies; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-16  2:33 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, john.mcnamara, michalx.k.jastrzebski, Tetsuya Mukawa

This patch fixes wrong error checking of rte_eal_parse_devargs_str().
Currently, a return value of strdup() is wrongly checked.

Fixes: 0fe11ec592b2 ("eal: add vdev init and uninit")
Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/common/eal_common_devargs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 2bfe54a..e403717 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -58,7 +58,7 @@ rte_eal_parse_devargs_str(const char *devargs_str,
 		return -1;
 
 	*drvname = strdup(devargs_str);
-	if (drvname == NULL)
+	if (*drvname == NULL)
 		return -1;
 
 	/* set the first ',' to '\0' to split name and arguments */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2] eal: Fix wrong resource release while unmapping pci devices
  2016-06-15  3:27   ` [dpdk-dev] [PATCH] eal: Fix wrong resource release of pci_uio_unmap() Tetsuya Mukawa
  2016-06-15 15:01     ` David Marchand
@ 2016-06-16  2:33     ` Tetsuya Mukawa
  2016-06-17 12:12       ` David Marchand
  1 sibling, 1 reply; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-16  2:33 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, john.mcnamara, michalx.k.jastrzebski, Tetsuya Mukawa

This patch fixes wrong resource release of pci_uio_unmap().
The 'path' member of mapped_pci_resource structure is allocated by
primary process, but currently it will be freed by both primary
and secondary process.
The patch fixes to be freed by only primary process.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/common/eal_common_pci_uio.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
index f062e81..488b6dd 100644
--- a/lib/librte_eal/common/eal_common_pci_uio.c
+++ b/lib/librte_eal/common/eal_common_pci_uio.c
@@ -159,7 +159,9 @@ pci_uio_unmap(struct mapped_pci_resource *uio_res)
 	for (i = 0; i != uio_res->nb_maps; i++) {
 		pci_unmap_resource(uio_res->maps[i].addr,
 				(size_t)uio_res->maps[i].size);
-		rte_free(uio_res->maps[i].path);
+
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+			rte_free(uio_res->maps[i].path);
 	}
 }
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2] eal: Fix resource leak while secondary process maps pci devices
  2016-06-15  3:26   ` [dpdk-dev] [PATCH] eal: Fix resource leak of pci_uio_map_secondary() Tetsuya Mukawa
  2016-06-15 14:45     ` David Marchand
@ 2016-06-16  2:33     ` Tetsuya Mukawa
  2016-06-17 12:28       ` David Marchand
  1 sibling, 1 reply; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-16  2:33 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, john.mcnamara, michalx.k.jastrzebski, Tetsuya Mukawa

This patch fixes resource leak of pci_uio_map_secondary().
If pci_map_resource() succeeds but mapped address is different from an
address primary process mapped, this should be error.
Then the addresses secondary process mapped should be freed.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/common/eal_common_pci_uio.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
index 488b6dd..83314c3 100644
--- a/lib/librte_eal/common/eal_common_pci_uio.c
+++ b/lib/librte_eal/common/eal_common_pci_uio.c
@@ -53,7 +53,7 @@ EAL_REGISTER_TAILQ(rte_uio_tailq)
 static int
 pci_uio_map_secondary(struct rte_pci_device *dev)
 {
-	int fd, i;
+	int fd, i, j;
 	struct mapped_pci_resource *uio_res;
 	struct mapped_pci_res_list *uio_res_list =
 			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
@@ -85,6 +85,17 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 					"Cannot mmap device resource file %s to address: %p\n",
 					uio_res->maps[i].path,
 					uio_res->maps[i].addr);
+				if (mapaddr != MAP_FAILED) {
+					/* unmap addrs correctly mapped */
+					for (j = 0; j < i; j++)
+						pci_unmap_resource(
+							uio_res->maps[j].addr,
+							(size_t)uio_res->maps[j].size);
+
+					/* unmap addr wrongly mapped */
+					pci_unmap_resource(mapaddr,
+						(size_t)uio_res->maps[i].size);
+				}
 				return -1;
 			}
 		}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments
  2016-06-16  2:33 ` [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments Tetsuya Mukawa
  2016-06-15  3:26   ` [dpdk-dev] [PATCH] eal: Fix resource leak of pci_uio_map_secondary() Tetsuya Mukawa
  2016-06-15  3:27   ` [dpdk-dev] [PATCH] eal: Fix wrong resource release of pci_uio_unmap() Tetsuya Mukawa
@ 2016-06-17 12:10   ` David Marchand
  2016-06-20  8:49     ` Thomas Monjalon
  2 siblings, 1 reply; 18+ messages in thread
From: David Marchand @ 2016-06-17 12:10 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev, Mcnamara, John, michalx.k.jastrzebski

On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> This patch fixes wrong error checking of rte_eal_parse_devargs_str().
> Currently, a return value of strdup() is wrongly checked.
>
> Fixes: 0fe11ec592b2 ("eal: add vdev init and uninit")
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Forgot to pass this patch through scripts/check-git-log.sh script :

Wrong headline uppercase:
    eal: Fix wrong error checking while parsing device arguments


With this fixed, you can add my ack.

-- 
David Marchand

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: Fix wrong resource release while unmapping pci devices
  2016-06-16  2:33     ` [dpdk-dev] [PATCH v2] eal: Fix wrong resource release while unmapping pci devices Tetsuya Mukawa
@ 2016-06-17 12:12       ` David Marchand
  2016-06-20  8:50         ` Thomas Monjalon
  0 siblings, 1 reply; 18+ messages in thread
From: David Marchand @ 2016-06-17 12:12 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev, Mcnamara, John, michalx.k.jastrzebski

On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> This patch fixes wrong resource release of pci_uio_unmap().
> The 'path' member of mapped_pci_resource structure is allocated by
> primary process, but currently it will be freed by both primary
> and secondary process.
> The patch fixes to be freed by only primary process.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Wrong headline uppercase:
    eal: Fix wrong resource release while unmapping pci devices
Wrong headline lowercase:
    eal: Fix wrong resource release while unmapping pci devices
Missing 'Fixes' tag:
    eal: Fix wrong resource release while unmapping pci devices

Then you can add my ack.


-- 
David Marchand

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: Fix resource leak while secondary process maps pci devices
  2016-06-16  2:33     ` [dpdk-dev] [PATCH v2] eal: Fix resource leak while secondary process maps pci devices Tetsuya Mukawa
@ 2016-06-17 12:28       ` David Marchand
  2016-06-20  2:19         ` Tetsuya Mukawa
  0 siblings, 1 reply; 18+ messages in thread
From: David Marchand @ 2016-06-17 12:28 UTC (permalink / raw)
  To: Tetsuya Mukawa, Thomas Monjalon
  Cc: dev, Mcnamara, John, michalx.k.jastrzebski

On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> This patch fixes resource leak of pci_uio_map_secondary().
> If pci_map_resource() succeeds but mapped address is different from an
> address primary process mapped, this should be error.
> Then the addresses secondary process mapped should be freed.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

scripts/check-git-log.sh is not happy :

Wrong headline uppercase:
    eal: Fix resource leak while secondary process maps pci devices
Wrong headline lowercase:
    eal: Fix resource leak while secondary process maps pci devices
Headline too long:
    eal: Fix resource leak while secondary process maps pci devices
Missing 'Fixes' tag:
    eal: Fix resource leak while secondary process maps pci devices


checkpatch is not happy, but I think we can ignore it.

WARNING:LONG_LINE: line over 80 characters
#48: FILE: lib/librte_eal/common/eal_common_pci_uio.c:93:
+                            (size_t)uio_res->maps[j].size);


Anyways, looks good to me, Thomas, can you fix the commit logs of
those last 3 patches on eal ?
Thanks.

-- 
David Marchand

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: Fix resource leak while secondary process maps pci devices
  2016-06-17 12:28       ` David Marchand
@ 2016-06-20  2:19         ` Tetsuya Mukawa
  2016-06-20  8:50           ` Thomas Monjalon
  0 siblings, 1 reply; 18+ messages in thread
From: Tetsuya Mukawa @ 2016-06-20  2:19 UTC (permalink / raw)
  To: David Marchand
  Cc: Thomas Monjalon, dev, Mcnamara, John, michalx.k.jastrzebski

On 2016/06/17 21:28, David Marchand wrote:
> On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
>> This patch fixes resource leak of pci_uio_map_secondary().
>> If pci_map_resource() succeeds but mapped address is different from an
>> address primary process mapped, this should be error.
>> Then the addresses secondary process mapped should be freed.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> 
> scripts/check-git-log.sh is not happy :
> 
> Wrong headline uppercase:
>     eal: Fix resource leak while secondary process maps pci devices
> Wrong headline lowercase:
>     eal: Fix resource leak while secondary process maps pci devices
> Headline too long:
>     eal: Fix resource leak while secondary process maps pci devices
> Missing 'Fixes' tag:
>     eal: Fix resource leak while secondary process maps pci devices
> 
> 
> checkpatch is not happy, but I think we can ignore it.
> 
> WARNING:LONG_LINE: line over 80 characters
> #48: FILE: lib/librte_eal/common/eal_common_pci_uio.c:93:
> +                            (size_t)uio_res->maps[j].size);
> 
> 
> Anyways, looks good to me, Thomas, can you fix the commit logs of
> those last 3 patches on eal ?
> Thanks.
> 

Hi David,

I appreciate your checking.
Next time I will check check-git-log.sh before submitting.


Hi Thomas,

Could you please let me know if you need the fixed patches.

Best,
Tetsuya

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments
  2016-06-17 12:10   ` [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments David Marchand
@ 2016-06-20  8:49     ` Thomas Monjalon
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Monjalon @ 2016-06-20  8:49 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev, David Marchand, Mcnamara, John, michalx.k.jastrzebski

2016-06-17 14:10, David Marchand:
> On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> > This patch fixes wrong error checking of rte_eal_parse_devargs_str().
> > Currently, a return value of strdup() is wrongly checked.
> >
> > Fixes: 0fe11ec592b2 ("eal: add vdev init and uninit")
> > Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> 
> Forgot to pass this patch through scripts/check-git-log.sh script :
> 
> Wrong headline uppercase:
>     eal: Fix wrong error checking while parsing device arguments
> 
> 
> With this fixed, you can add my ack.

Applied, thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: Fix wrong resource release while unmapping pci devices
  2016-06-17 12:12       ` David Marchand
@ 2016-06-20  8:50         ` Thomas Monjalon
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Monjalon @ 2016-06-20  8:50 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: dev, David Marchand, Mcnamara, John, michalx.k.jastrzebski

2016-06-17 14:12, David Marchand:
> On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> > This patch fixes wrong resource release of pci_uio_unmap().
> > The 'path' member of mapped_pci_resource structure is allocated by
> > primary process, but currently it will be freed by both primary
> > and secondary process.
> > The patch fixes to be freed by only primary process.
> >
> > Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> 
> Wrong headline uppercase:
>     eal: Fix wrong resource release while unmapping pci devices
> Wrong headline lowercase:
>     eal: Fix wrong resource release while unmapping pci devices
> Missing 'Fixes' tag:
>     eal: Fix wrong resource release while unmapping pci devices
> 
> Then you can add my ack.

Applied, thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: Fix resource leak while secondary process maps pci devices
  2016-06-20  2:19         ` Tetsuya Mukawa
@ 2016-06-20  8:50           ` Thomas Monjalon
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Monjalon @ 2016-06-20  8:50 UTC (permalink / raw)
  To: Tetsuya Mukawa; +Cc: David Marchand, dev, Mcnamara, John, michalx.k.jastrzebski

2016-06-20 11:19, Tetsuya Mukawa:
> On 2016/06/17 21:28, David Marchand wrote:
> > On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> >> This patch fixes resource leak of pci_uio_map_secondary().
> >> If pci_map_resource() succeeds but mapped address is different from an
> >> address primary process mapped, this should be error.
> >> Then the addresses secondary process mapped should be freed.
> >>
> >> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> > 
> > scripts/check-git-log.sh is not happy :
> > 
> > Wrong headline uppercase:
> >     eal: Fix resource leak while secondary process maps pci devices
> > Wrong headline lowercase:
> >     eal: Fix resource leak while secondary process maps pci devices
> > Headline too long:
> >     eal: Fix resource leak while secondary process maps pci devices
> > Missing 'Fixes' tag:
> >     eal: Fix resource leak while secondary process maps pci devices
> > 
> > 
> > checkpatch is not happy, but I think we can ignore it.
> > 
> > WARNING:LONG_LINE: line over 80 characters
> > #48: FILE: lib/librte_eal/common/eal_common_pci_uio.c:93:
> > +                            (size_t)uio_res->maps[j].size);
> > 
> > 
> > Anyways, looks good to me, Thomas, can you fix the commit logs of
> > those last 3 patches on eal ?
> > Thanks.
> > 
> 
> Hi David,
> 
> I appreciate your checking.
> Next time I will check check-git-log.sh before submitting.
> 
> 
> Hi Thomas,
> 
> Could you please let me know if you need the fixed patches.

Applied, thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-06-20  8:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-15  3:26 [dpdk-dev] [PATCH] eal: Fix wrong error checking of rte_eal_parse_devargs_str() Tetsuya Mukawa
2016-06-15 14:48 ` David Marchand
2016-06-16  2:03   ` Tetsuya Mukawa
2016-06-16  2:33 ` [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments Tetsuya Mukawa
2016-06-15  3:26   ` [dpdk-dev] [PATCH] eal: Fix resource leak of pci_uio_map_secondary() Tetsuya Mukawa
2016-06-15 14:45     ` David Marchand
2016-06-16  2:03       ` Tetsuya Mukawa
2016-06-16  2:33     ` [dpdk-dev] [PATCH v2] eal: Fix resource leak while secondary process maps pci devices Tetsuya Mukawa
2016-06-17 12:28       ` David Marchand
2016-06-20  2:19         ` Tetsuya Mukawa
2016-06-20  8:50           ` Thomas Monjalon
2016-06-15  3:27   ` [dpdk-dev] [PATCH] eal: Fix wrong resource release of pci_uio_unmap() Tetsuya Mukawa
2016-06-15 15:01     ` David Marchand
2016-06-16  2:33     ` [dpdk-dev] [PATCH v2] eal: Fix wrong resource release while unmapping pci devices Tetsuya Mukawa
2016-06-17 12:12       ` David Marchand
2016-06-20  8:50         ` Thomas Monjalon
2016-06-17 12:10   ` [dpdk-dev] [PATCH v2] eal: Fix wrong error checking while parsing device arguments David Marchand
2016-06-20  8:49     ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).