diff mbox series

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

Message ID	20241009211302.177471-12-hernan.vargas@intel.com (mailing list archive)
State	Accepted, archived
Delegated to:	Maxime Coquelin
Headers	From: Hernan Vargas <hernan.vargas@intel.com> To: dev@dpdk.org, gakhil@marvell.com, trix@redhat.com, maxime.coquelin@redhat.com Cc: nicolas.chautru@intel.com, qi.z.zhang@intel.com, Hernan Vargas <hernan.vargas@intel.com> Subject: [PATCH v3 11/12] baseband/acc: rte free refactor Date: Wed, 9 Oct 2024 14:13:01 -0700 Message-Id: <20241009211302.177471-12-hernan.vargas@intel.com> In-Reply-To: <20241009211302.177471-1-hernan.vargas@intel.com> References: <20241009211302.177471-1-hernan.vargas@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: list Errors-To: dev-bounces@dpdk.org
Series	acc baseband PMD fix and updates for 24.11 \| [v3,00/12] acc baseband PMD fix and updates for 24.11 [v3,01/12] baseband/acc: fix access to deallocated mem [v3,02/12] baseband/acc: fix soft output bypass RM [v3,03/12] baseband/acc: queue allocation refactor [v3,04/12] baseband/acc: configure max queues per device [v3,05/12] baseband/acc: future proof structure comparison [v3,06/12] baseband/acc: enhance SW ring alignment [v3,07/12] baseband/acc: algorithm tuning for LDPC decoder [v3,08/12] baseband/acc: remove check on HARQ memory [v3,09/12] baseband/acc: reset ring data valid bit [v3,10/12] baseband/acc: cosmetic changes [v3,11/12] baseband/acc: rte free refactor [v3,12/12] baseband/acc: clean up of VRB1 capabilities

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

diff mbox series

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. */