[v7, net-next 01/10] bng_en: make bnge_alloc_ring() self-unwind on failure

Bhargava Marreddy posted 10 patches 2 weeks, 6 days ago
There is a newer version of this series
[v7, net-next 01/10] bng_en: make bnge_alloc_ring() self-unwind on failure
Posted by Bhargava Marreddy 2 weeks, 6 days ago
Ensure bnge_alloc_ring() frees any intermediate allocations
when it fails. This enables later patches to rely on this
self-unwinding behavior.

Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnge/bnge_rmem.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
index 52ada65943a..98b4e9f55bc 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.c
@@ -95,7 +95,7 @@ int bnge_alloc_ring(struct bnge_dev *bd, struct bnge_ring_mem_info *rmem)
 						     &rmem->dma_arr[i],
 						     GFP_KERNEL);
 		if (!rmem->pg_arr[i])
-			return -ENOMEM;
+			goto err_free_ring;
 
 		if (rmem->ctx_mem)
 			bnge_init_ctx_mem(rmem->ctx_mem, rmem->pg_arr[i],
@@ -116,10 +116,13 @@ int bnge_alloc_ring(struct bnge_dev *bd, struct bnge_ring_mem_info *rmem)
 	if (rmem->vmem_size) {
 		*rmem->vmem = vzalloc(rmem->vmem_size);
 		if (!(*rmem->vmem))
-			return -ENOMEM;
+			goto err_free_ring;
 	}
-
 	return 0;
+
+err_free_ring:
+	bnge_free_ring(bd, rmem);
+	return -ENOMEM;
 }
 
 static int bnge_alloc_ctx_one_lvl(struct bnge_dev *bd,
-- 
2.47.3
Re: [v7, net-next 01/10] bng_en: make bnge_alloc_ring() self-unwind on failure
Posted by Simon Horman 2 weeks, 2 days ago
On Fri, Sep 12, 2025 at 01:04:56AM +0530, Bhargava Marreddy wrote:
> Ensure bnge_alloc_ring() frees any intermediate allocations
> when it fails. This enables later patches to rely on this
> self-unwinding behavior.
> 
> Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
> Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>

Without this patch(set), does the code correctly release resources on error?

If not, I think this should be considered a fix for net with appropriate
Fixes tag(s).

...
Re: [v7, net-next 01/10] bng_en: make bnge_alloc_ring() self-unwind on failure
Posted by Bhargava Chenna Marreddy 2 weeks ago
On Tue, Sep 16, 2025 at 8:43 PM Simon Horman <horms@kernel.org> wrote:
>
> On Fri, Sep 12, 2025 at 01:04:56AM +0530, Bhargava Marreddy wrote:
> > Ensure bnge_alloc_ring() frees any intermediate allocations
> > when it fails. This enables later patches to rely on this
> > self-unwinding behavior.
> >
> > Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
> > Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
> > Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
>
> Without this patch(set), does the code correctly release resources on error?
>
> If not, I think this should be considered a fix for net with appropriate
> Fixes tag(s).

Thanks for your feedback, Simon. This patch doesn't introduce a fix;
the code already frees resources correctly.
Instead, it modifies error handling by changing from caller-unwind to
self-unwind within this function

>
> ...
Re: [v7, net-next 01/10] bng_en: make bnge_alloc_ring() self-unwind on failure
Posted by Simon Horman 2 weeks ago
On Thu, Sep 18, 2025 at 03:20:09PM +0530, Bhargava Chenna Marreddy wrote:
> On Tue, Sep 16, 2025 at 8:43 PM Simon Horman <horms@kernel.org> wrote:
> >
> > On Fri, Sep 12, 2025 at 01:04:56AM +0530, Bhargava Marreddy wrote:
> > > Ensure bnge_alloc_ring() frees any intermediate allocations
> > > when it fails. This enables later patches to rely on this
> > > self-unwinding behavior.
> > >
> > > Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
> > > Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
> > > Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
> >
> > Without this patch(set), does the code correctly release resources on error?
> >
> > If not, I think this should be considered a fix for net with appropriate
> > Fixes tag(s).
> 
> Thanks for your feedback, Simon. This patch doesn't introduce a fix;
> the code already frees resources correctly.
> Instead, it modifies error handling by changing from caller-unwind to
> self-unwind within this function

Thanks for the clarification.
In that case, this looks good to me.