[PATCH] net: ethernet: sun4i-emac: free dma descriptor

Conley Lee posted 1 patch 4 weeks, 1 day ago
There is a newer version of this series
drivers/net/ethernet/allwinner/sun4i-emac.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] net: ethernet: sun4i-emac: free dma descriptor
Posted by Conley Lee 4 weeks, 1 day ago
In the current implementation of the sun4i-emac driver, when using DMA to
receive data packets, the descriptor for the current DMA request is not
released in the rx_done_callback.

Fix this by properly releasing the descriptor.

Fixes: 47869e82c8b8 ("sun4i-emac.c: add dma support")
Signed-off-by: Conley Lee <conleylee@foxmail.com>
---
 drivers/net/ethernet/allwinner/sun4i-emac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 2f516b950..2f990d0a5 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -237,6 +237,7 @@ emac_alloc_dma_req(struct emac_board_info *db,
 
 static void emac_free_dma_req(struct emac_dma_req *req)
 {
+	dmaengine_desc_free(req->desc);
 	kfree(req);
 }
 
-- 
2.25.1
Re: [PATCH] net: ethernet: sun4i-emac: free dma descriptor
Posted by Jakub Kicinski 4 weeks ago
On Wed,  3 Sep 2025 15:49:39 +0800 Conley Lee wrote:
> In the current implementation of the sun4i-emac driver, when using DMA to
> receive data packets, the descriptor for the current DMA request is not
> released in the rx_done_callback.
> 
> Fix this by properly releasing the descriptor.

Reading the docs, it appears that the need to free the desc is tied to
setting descriptor reuse flag. Which this driver does not do. So I'm
unclear why this is needed, maybe the dma engine driver is doing
something strange?

Could you repost this, CC the dmaengine ML, Vinod and the appropriate
SoC maintainers?
[PATCH] net: ethernet: sun4i-emac: free dma descriptor
Posted by Conley Lee 3 weeks, 4 days ago
In the current implementation of the sun4i-emac driver, when using DMA to
receive data packets, the descriptor for the current DMA request is not
released in the rx_done_callback.

Fix this by properly releasing the descriptor.

Fixes: 47869e82c8b8 ("sun4i-emac.c: add dma support")
Signed-off-by: Conley Lee <conleylee@foxmail.com>
---
 drivers/net/ethernet/allwinner/sun4i-emac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 2f516b950..2f990d0a5 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -237,6 +237,7 @@ emac_alloc_dma_req(struct emac_board_info *db,
 
 static void emac_free_dma_req(struct emac_dma_req *req)
 {
+	dmaengine_desc_free(req->desc);
 	kfree(req);
 }
 
-- 
2.25.1
Re: [PATCH] net: ethernet: sun4i-emac: free dma descriptor
Posted by Jakub Kicinski 3 weeks, 3 days ago
On Sun,  7 Sep 2025 19:29:30 +0800 Conley Lee wrote:
> In the current implementation of the sun4i-emac driver, when using DMA to
> receive data packets, the descriptor for the current DMA request is not
> released in the rx_done_callback.

I wish you elaborated more on the reuse flag not being set :\

> Fix this by properly releasing the descriptor.
> 
> Fixes: 47869e82c8b8 ("sun4i-emac.c: add dma support")
> Signed-off-by: Conley Lee <conleylee@foxmail.com>

Hi Vinod, could you TAL? Is this fix legit or there's something wrong
with how the DMA engine API is used on this platform?
https://lore.kernel.org/all/tencent_D434891410B0717BB0BDCB1434969E6EB50A@qq.com/
Re: [PATCH] net: ethernet: sun4i-emac: free dma descriptor
Posted by 李克斯 3 weeks, 3 days ago
Thank you for the suggestion. I've reviewed the documentation, and setting the reuse flag while reusing descriptors might be a good optimization. I'll make the changes and run some tests. If everything works well, I'll submit a new patch.

> 2025年9月9日 04:26,Jakub Kicinski <kuba@kernel.org> 写道:
> 
> On Sun,  7 Sep 2025 19:29:30 +0800 Conley Lee wrote:
>> In the current implementation of the sun4i-emac driver, when using DMA to
>> receive data packets, the descriptor for the current DMA request is not
>> released in the rx_done_callback.
> 
> I wish you elaborated more on the reuse flag not being set :\
> 
>> Fix this by properly releasing the descriptor.
>> 
>> Fixes: 47869e82c8b8 ("sun4i-emac.c: add dma support")
>> Signed-off-by: Conley Lee <conleylee@foxmail.com>
> 
> Hi Vinod, could you TAL? Is this fix legit or there's something wrong
> with how the DMA engine API is used on this platform?
> https://lore.kernel.org/all/tencent_D434891410B0717BB0BDCB1434969E6EB50A@qq.com/
Re: [PATCH] net: ethernet: sun4i-emac: free dma descriptor
Posted by Jakub Kicinski 3 weeks, 2 days ago
On Tue, 9 Sep 2025 14:36:42 +0800 李克斯 wrote:
> Thank you for the suggestion. I've reviewed the documentation, and
> setting the reuse flag while reusing descriptors might be a good
> optimization. I'll make the changes and run some tests. If everything
> works well, I'll submit a new patch.

To be clear if you're saying the driver is buggy and can crash right
now we need to fix it first and then optimize it later, as separate
commits. So that LTS kernels can backport the fix.

The questions I'm asking are because I don't understand whether the
upstream kernel is buggy and how exactly..
Re: [PATCH] net: ethernet: sun4i-emac: free dma descriptor
Posted by Conley Lee 3 weeks, 1 day ago
On 09/09/25 at 06:15下午, Jakub Kicinski wrote:
> Date: Tue, 9 Sep 2025 18:15:47 -0700
> From: Jakub Kicinski <kuba@kernel.org>
> To: 李克斯 <conleylee@foxmail.com>
> Cc: vkoul@kernel.org, davem@davemloft.net, wens@csie.org,
>  mripard@kernel.org, netdev@vger.kernel.org, dmaengine@vger.kernel.org,
>  linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] net: ethernet: sun4i-emac: free dma descriptor
> 
> On Tue, 9 Sep 2025 14:36:42 +0800 李克斯 wrote:
> > Thank you for the suggestion. I've reviewed the documentation, and
> > setting the reuse flag while reusing descriptors might be a good
> > optimization. I'll make the changes and run some tests. If everything
> > works well, I'll submit a new patch.
> 
> To be clear if you're saying the driver is buggy and can crash right
> now we need to fix it first and then optimize it later, as separate
> commits. So that LTS kernels can backport the fix.
> 
> The questions I'm asking are because I don't understand whether the
> upstream kernel is buggy and how exactly..

After carefully reviewing the documentation and how other drivers use
dmaengine, I think that if the reuse flag is not set for the descriptor,
manual release is unnecessary. Therefore, the current driver implementation
does not contain a bug. This fix patch was a mistake.

Thank you for your thorough review.