[v3,11/12] baseband/acc: rte free refactor

Message ID 20241009211302.177471-12-hernan.vargas@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Maxime Coquelin
Headers
Series acc baseband PMD fix and updates for 24.11 |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Hernan Vargas Oct. 9, 2024, 9:13 p.m. UTC
Refactor to explicitly set pointer to NULL after free to avoid double
free.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 23 +++++++------
 drivers/baseband/acc/rte_vrb_pmd.c    | 48 +++++++++++++++------------
 2 files changed, 39 insertions(+), 32 deletions(-)
  

Comments

Maxime Coquelin Oct. 14, 2024, 11:03 a.m. UTC | #1
I would rename the title to:

"baseband/acc: refactor resources freeing"

I can fix while applying.

On 10/9/24 23:13, Hernan Vargas wrote:
> Refactor to explicitly set pointer to NULL after free to avoid double
> free.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 23 +++++++------
>   drivers/baseband/acc/rte_vrb_pmd.c    | 48 +++++++++++++++------------
>   2 files changed, 39 insertions(+), 32 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  

Patch

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index c82a0b6cc174..d33e42c8070b 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -564,6 +564,7 @@  acc100_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 	d->tail_ptrs = NULL;
 free_sw_rings:
 	rte_free(d->sw_rings_base);
+	d->sw_rings_base = NULL;
 	d->sw_rings = NULL;
 
 	return ret;
@@ -593,6 +594,7 @@  acc100_intr_enable(struct rte_bbdev *dev)
 					"Couldn't enable interrupts for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 		ret = rte_intr_callback_register(dev->intr_handle,
@@ -602,6 +604,7 @@  acc100_intr_enable(struct rte_bbdev *dev)
 					"Couldn't register interrupt callback for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 
@@ -619,16 +622,15 @@  acc100_dev_close(struct rte_bbdev *dev)
 {
 	struct acc_device *d = dev->data->dev_private;
 	acc100_check_ir(d);
-	if (d->sw_rings_base != NULL) {
-		rte_free(d->tail_ptrs);
-		rte_free(d->info_ring);
-		rte_free(d->sw_rings_base);
-		rte_free(d->harq_layout);
-		d->sw_rings_base = NULL;
-		d->tail_ptrs = NULL;
-		d->info_ring = NULL;
-		d->harq_layout = NULL;
-	}
+	rte_free(d->tail_ptrs);
+	rte_free(d->info_ring);
+	rte_free(d->sw_rings_base);
+	rte_free(d->harq_layout);
+	d->tail_ptrs = NULL;
+	d->info_ring = NULL;
+	d->sw_rings_base = NULL;
+	d->sw_rings = NULL;
+	d->harq_layout = NULL;
 	/* Ensure all in flight HW transactions are completed */
 	usleep(ACC_LONG_WAIT);
 	return 0;
@@ -4235,6 +4237,7 @@  poweron_cleanup(struct rte_bbdev *bbdev, struct acc_device *d,
 	rte_bbdev_log(INFO, "Number of 5GUL engines %d", numEngines);
 
 	rte_free(d->sw_rings_base);
+	d->sw_rings_base = NULL;
 	usleep(ACC_LONG_WAIT);
 }
 
diff --git a/drivers/baseband/acc/rte_vrb_pmd.c b/drivers/baseband/acc/rte_vrb_pmd.c
index c0464d20c641..03df270af1cf 100644
--- a/drivers/baseband/acc/rte_vrb_pmd.c
+++ b/drivers/baseband/acc/rte_vrb_pmd.c
@@ -745,10 +745,13 @@  vrb_setup_queues(struct rte_bbdev *dev, uint16_t num_queues, int socket_id)
 free_sw_rings:
 	if (d->device_variant == VRB1_VARIANT) {
 		rte_free(d->sw_rings_base);
+		d->sw_rings_base = NULL;
 		d->sw_rings = NULL;
 	} else if (d->device_variant == VRB2_VARIANT) {
-		for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++)
+		for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++) {
 			rte_free(d->sw_rings_array[i]);
+			d->sw_rings_array[i] = 0;
+		}
 	}
 
 	return ret;
@@ -786,6 +789,7 @@  vrb_intr_enable(struct rte_bbdev *dev)
 					"Couldn't enable interrupts for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 		ret = rte_intr_callback_register(dev->intr_handle,
@@ -795,6 +799,7 @@  vrb_intr_enable(struct rte_bbdev *dev)
 					"Couldn't register interrupt callback for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 
@@ -849,6 +854,7 @@  vrb_intr_enable(struct rte_bbdev *dev)
 					"Couldn't enable interrupts for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 		ret = rte_intr_callback_register(dev->intr_handle,
@@ -858,6 +864,7 @@  vrb_intr_enable(struct rte_bbdev *dev)
 					"Couldn't register interrupt callback for device: %s",
 					dev->data->name);
 			rte_free(d->info_ring);
+			d->info_ring = NULL;
 			return ret;
 		}
 
@@ -878,28 +885,25 @@  vrb_dev_close(struct rte_bbdev *dev)
 
 	vrb_check_ir(d);
 	if (d->device_variant == VRB1_VARIANT) {
-		if (d->sw_rings_base != NULL) {
-			rte_free(d->tail_ptrs);
-			rte_free(d->info_ring);
-			rte_free(d->sw_rings_base);
-			rte_free(d->harq_layout);
-			d->tail_ptrs = NULL;
-			d->info_ring = NULL;
-			d->sw_rings_base = NULL;
-			d->harq_layout = NULL;
-		}
+		rte_free(d->tail_ptrs);
+		rte_free(d->info_ring);
+		rte_free(d->sw_rings_base);
+		rte_free(d->harq_layout);
+		d->tail_ptrs = NULL;
+		d->info_ring = NULL;
+		d->sw_rings_base = NULL;
+		d->sw_rings = NULL;
+		d->harq_layout = NULL;
 	} else if (d->device_variant == VRB2_VARIANT) {
-		if (d->sw_rings_array[1] != NULL) {
-			rte_free(d->tail_ptrs);
-			rte_free(d->info_ring);
-			rte_free(d->harq_layout);
-			d->tail_ptrs = NULL;
-			d->info_ring = NULL;
-			d->harq_layout = NULL;
-			for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++) {
-				rte_free(d->sw_rings_array[i]);
-				d->sw_rings_array[i] = NULL;
-			}
+		rte_free(d->tail_ptrs);
+		rte_free(d->info_ring);
+		rte_free(d->harq_layout);
+		d->tail_ptrs = NULL;
+		d->info_ring = NULL;
+		d->harq_layout = NULL;
+		for (i = 0; i <= RTE_BBDEV_OP_MLDTS; i++) {
+			rte_free(d->sw_rings_array[i]);
+			d->sw_rings_array[i] = NULL;
 		}
 	}
 	/* Ensure all in flight HW transactions are completed. */