Skip to contentSkip to conversation

raw/ifpga: fix ifpga devices cleanup function

PATCH Applied Wei Huang 5 participants 8 messages Jan 26, 2022 – Jan 28, 2022

lore
Share & tools
lore

Wei Huang

author
#0 lore raw

PATCH 1/1 raw/ifpga: fix ifpga devices cleanup function

Use rte_dev_remove() to replace rte_rawdev_pmd_release() in ifpga_rawdev_cleanup(), resources occupied by ifpga raw devices such as threads can be released correctly.

Fixes f724a802 ("raw/ifpga: add miscellaneous APIs")
Signed-off-by Wei Huang <[email protected]>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/raw/ifpga/ifpga_rawdev.c +3 -1
index fdf3c23..88c38aa 100644
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct rte_rawdev *dev, int port,
 void ifpga_rawdev_cleanup(void)
 {
 	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
 	unsigned int i;
 
 	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 		dev = &ifpga_rawdevices[i];
 		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
 			dev->rawdev = NULL;
 		}
 	}
Signature · 1 line
1.8.3.1
Acked-by on v1 1/1
#1 lore raw
Quoted 33 lines

-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, January 26, 2022 11:30 AM
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Yigit, Ferruh <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1] raw/ifpga: fix ifpga devices cleanup function

Use rte_dev_remove() to replace rte_rawdev_pmd_release() in ifpga_rawdev_cleanup(), resources occupied by ifpga raw devices such as threads can be released correctly.

Fixes f724a802 ("raw/ifpga: add miscellaneous APIs")
Signed-off-by Wei Huang <[email protected]>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/raw/ifpga/ifpga_rawdev.c +1 -0
index fdf3c23..88c38aa 100644
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct
 rte_rawdev *dev, int port,  void ifpga_rawdev_cleanup(void)  {
 	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
 	unsigned int i;

drivers/raw/ifpga/ifpga_rawdev.c int ifpga_rawdev_partial_reconfigure(struct · line 1798

Quoted 10 lines
drivers/raw/ifpga/ifpga_rawdev.c +2 -2
 	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 		dev = &ifpga_rawdevices[i];
 		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
 			dev->rawdev = NULL;
 		}
 	}
--

It looks good for me.

Acked-by Tianfei Zhang <[email protected]>
Acked-by on v1 1/1
#2 lore raw
In reply to Wei Huang

Hi,

Quoted 41 lines

-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, January 26, 2022 11:30
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Yigit, Ferruh <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1] raw/ifpga: fix ifpga devices cleanup function

Use rte_dev_remove() to replace rte_rawdev_pmd_release() in ifpga_rawdev_cleanup(), resources occupied by ifpga raw devices such as threads can be released correctly.

Fixes f724a802 ("raw/ifpga: add miscellaneous APIs")
Signed-off-by Wei Huang <[email protected]>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/raw/ifpga/ifpga_rawdev.c +3 -2
index fdf3c23..88c38aa 100644
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct
 rte_rawdev *dev, int port,  void ifpga_rawdev_cleanup(void)  {
 	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
 	unsigned int i;
 
 	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 		dev = &ifpga_rawdevices[i];
 		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
 			dev->rawdev = NULL;
 		}
 	}
--
Quoted 1 line

1.8.3.1

Acked-by Rosen Xu <[email protected]>
Applied
#3 lore raw
Quoted 44 lines

-----Original Message-----
From: Xu, Rosen <[email protected]>
Sent: Wednesday, January 26, 2022 3:08 PM
To: Huang, Wei <[email protected]>; [email protected]; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Yigit, Ferruh <[email protected]>
Subject: RE: [PATCH v1] raw/ifpga: fix ifpga devices cleanup function

Hi,

Quoted 31 lines

-----Original Message-----
From: Huang, Wei <[email protected]>
Sent: Wednesday, January 26, 2022 11:30
To: [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>; Yigit, Ferruh <[email protected]>; Huang, Wei <[email protected]>
Subject: [PATCH v1] raw/ifpga: fix ifpga devices cleanup function

Use rte_dev_remove() to replace rte_rawdev_pmd_release() in ifpga_rawdev_cleanup(), resources occupied by ifpga raw devices such as threads can be released correctly.

Fixes f724a802 ("raw/ifpga: add miscellaneous APIs")
Signed-off-by Wei Huang <[email protected]>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/raw/ifpga/ifpga_rawdev.c +1 -0
index fdf3c23..88c38aa 100644
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct
 rte_rawdev *dev, int port,  void ifpga_rawdev_cleanup(void)  {
 	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
 	unsigned int i;
Quoted 13 lines
Quoted 11 lines
drivers/raw/ifpga/ifpga_rawdev.c +2 -2
 	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 		dev = &ifpga_rawdevices[i];
 		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
 			dev->rawdev = NULL;
 		}
 	}
--

1.8.3.1

Acked-by Rosen Xu <[email protected]>

Applied to dpdk-next-net-intel.

Thanks
Qi

#4 lore raw
In reply to Wei Huang
Quoted 4 lines · Wei Huang

Use rte_dev_remove() to replace rte_rawdev_pmd_release() in ifpga_rawdev_cleanup(), resources occupied by ifpga raw devices such as threads can be released correctly.

As far as I understand you are fixing an issue that not all resources are released, is this correct?
What are these not released resources?

And 'rte_rawdev_pmd_release()' rawdev API seems intended to do the cleanup, is it expected that some resources are not freed after this call, or should we fix that API?
If the device remove API needs to be used, what is the point of 'rte_rawdev_pmd_release()' API?

cc'ed rawdev maintainers for comment.

Quoted 27 lines
Fixes f724a802 ("raw/ifpga: add miscellaneous APIs")
Signed-off-by Wei Huang <[email protected]>
---
  drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
drivers/raw/ifpga/ifpga_rawdev.c +3 -1
index fdf3c23..88c38aa 100644
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct rte_rawdev *dev, int port,
  void ifpga_rawdev_cleanup(void)
  {
  	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
  	unsigned int i;
 
  	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
  		dev = &ifpga_rawdevices[i];
  		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
  			dev->rawdev = NULL;
  		}
  	}
author
#5 lore raw

Hi,

Quoted 19 lines

-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Wednesday, January 26, 2022 21:25
To: Huang, Wei <[email protected]>; [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>; Nipun Gupta <[email protected]>; Hemant Agrawal <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>
Subject: Re: [PATCH v1] raw/ifpga: fix ifpga devices cleanup function

Quoted 4 lines · Wei Huang

Use rte_dev_remove() to replace rte_rawdev_pmd_release() in ifpga_rawdev_cleanup(), resources occupied by ifpga raw devices such as threads can be released correctly.

As far as I understand you are fixing an issue that not all resources are released, is this correct?
What are these not released resources?

Quoted 7 lines

And 'rte_rawdev_pmd_release()' rawdev API seems intended to do the cleanup, is it expected that some resources are not freed after this call, or should we fix that API?
If the device remove API needs to be used, what is the point of 'rte_rawdev_pmd_release()' API?

cc'ed rawdev maintainers for comment.

Yes, this patch is to release all the resources of ifpga_rawdev after testpmd exit, the not released resources are interrupt and thread.

rte_rawdev_pmd_release implemented in ifpga_rawdev only release memory allocated by ifpga driver, that's the expected behavior.

I think it's a simple and safe way to release resources completely by calling rte_dev_remove.

Quoted 30 lines

drivers/raw/ifpga/ifpga_rawdev.c int ifpga_rawdev_partial_reconfigure(struct

Quoted 14 lines
Fixes f724a802 ("raw/ifpga: add miscellaneous APIs")
Signed-off-by Wei Huang <[email protected]>
---
  drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
drivers/raw/ifpga/ifpga_rawdev.c +0 -0
index fdf3c23..88c38aa 100644
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct

rte_rawdev *dev, int port,

Quoted 15 lines
drivers/raw/ifpga/ifpga_rawdev.c +3 -1
  void ifpga_rawdev_cleanup(void)
  {
  	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
  	unsigned int i;
 
  	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
  		dev = &ifpga_rawdevices[i];
  		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
  			dev->rawdev = NULL;
  		}
  	}
#6 lore raw
Quoted 27 lines · Wei

Hi,

Quoted 25 lines

-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Wednesday, January 26, 2022 21:25
To: Huang, Wei <[email protected]>; [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>; Nipun Gupta <[email protected]>; Hemant Agrawal <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>
Subject: Re: [PATCH v1] raw/ifpga: fix ifpga devices cleanup function

Quoted 4 lines · Wei Huang

Use rte_dev_remove() to replace rte_rawdev_pmd_release() in ifpga_rawdev_cleanup(), resources occupied by ifpga raw devices such as threads can be released correctly.

As far as I understand you are fixing an issue that not all resources are released, is this correct?
What are these not released resources?

And 'rte_rawdev_pmd_release()' rawdev API seems intended to do the cleanup, is it expected that some resources are not freed after this call, or should we fix that API?
If the device remove API needs to be used, what is the point of 'rte_rawdev_pmd_release()' API?

Quoted 8 lines
Quoted 1 line

cc'ed rawdev maintainers for comment.

Yes, this patch is to release all the resources of ifpga_rawdev after testpmd exit, the not released resources are interrupt and thread.

rte_rawdev_pmd_release implemented in ifpga_rawdev only release memory allocated by ifpga driver, that's the expected behavior.

I think it's a simple and safe way to release resources completely by calling rte_dev_remove.

If device hot remove is better option, why 'rte_rawdev_pmd_release()' API exists?

Quoted 31 lines
Quoted 30 lines

drivers/raw/ifpga/ifpga_rawdev.c int ifpga_rawdev_partial_reconfigure(struct

Quoted 14 lines
Fixes f724a802 ("raw/ifpga: add miscellaneous APIs")
Signed-off-by Wei Huang <[email protected]>
---
   drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)
drivers/raw/ifpga/ifpga_rawdev.c +0 -0
index fdf3c23..88c38aa 100644
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct

rte_rawdev *dev, int port,

Quoted 15 lines
drivers/raw/ifpga/ifpga_rawdev.c +3 -1
   void ifpga_rawdev_cleanup(void)
   {
   	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
   	unsigned int i;
 
   	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
   		dev = &ifpga_rawdevices[i];
   		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
   			dev->rawdev = NULL;
   		}
   	}
author
#7 lore raw

Hi,

Quoted 43 lines

-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Thursday, January 27, 2022 20:34
To: Huang, Wei <[email protected]>; [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>; Nipun Gupta <[email protected]>; Hemant Agrawal <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>
Subject: Re: [PATCH v1] raw/ifpga: fix ifpga devices cleanup function

Quoted 27 lines · Wei

Hi,

Quoted 25 lines

-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Wednesday, January 26, 2022 21:25
To: Huang, Wei <[email protected]>; [email protected]; Xu, Rosen <[email protected]>; Zhang, Qi Z <[email protected]>; Nipun Gupta <[email protected]>; Hemant Agrawal <[email protected]>
Cc: [email protected]; Zhang, Tianfei <[email protected]>
Subject: Re: [PATCH v1] raw/ifpga: fix ifpga devices cleanup function

Quoted 4 lines · Wei Huang

Use rte_dev_remove() to replace rte_rawdev_pmd_release() in ifpga_rawdev_cleanup(), resources occupied by ifpga raw devices such as threads can be released correctly.

As far as I understand you are fixing an issue that not all resources are released, is this correct?
What are these not released resources?

And 'rte_rawdev_pmd_release()' rawdev API seems intended to do the cleanup, is it expected that some resources are not freed after this call, or should we fix that API?
If the device remove API needs to be used, what is the point of 'rte_rawdev_pmd_release()' API?

Quoted 3 lines
Quoted 1 line

cc'ed rawdev maintainers for comment.

Yes, this patch is to release all the resources of ifpga_rawdev after testpmd

exit, the not released resources are interrupt and thread.

Quoted 2 lines

rte_rawdev_pmd_release implemented in ifpga_rawdev only release

Quoted 8 lines

memory allocated by ifpga driver, that's the expected behavior.

Quoted 2 lines

I think it's a simple and safe way to release resources completely by calling

rte_dev_remove.

Quoted 1 line

If device hot remove is better option, why 'rte_rawdev_pmd_release()' API exists?

Agree, let me try to free all resources in rte_rawdev_pmd_release().

Quoted 32 lines
Quoted 32 lines
Quoted 31 lines

drivers/raw/ifpga/ifpga_rawdev.c int ifpga_rawdev_partial_reconfigure(struct

Quoted 15 lines
Fixes f724a802 ("raw/ifpga: add miscellaneous APIs")
Signed-off-by Wei Huang <[email protected]>
---
   drivers/raw/ifpga/ifpga_rawdev.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)
drivers/raw/ifpga/ifpga_rawdev.c +0 -0
index fdf3c23..88c38aa 100644
@@ -1787,12 +1787,14 @@ int ifpga_rawdev_partial_reconfigure(struct

rte_rawdev *dev, int port,

Quoted 15 lines
drivers/raw/ifpga/ifpga_rawdev.c +3 -1
   void ifpga_rawdev_cleanup(void)
   {
   	struct ifpga_rawdev *dev;
+	struct rte_rawdev *rdev;
   	unsigned int i;
 
   	for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
   		dev = &ifpga_rawdevices[i];
   		if (dev->rawdev) {
-			rte_rawdev_pmd_release(dev->rawdev);
+			rdev = dev->rawdev;
+			rte_dev_remove(rdev->device);
   			dev->rawdev = NULL;
   		}
   	}