* [PATCH net] net: mvneta_bm: fix gen_pool_free address for BPPI
@ 2026-08-05 6:14 Chenguang Zhao
2026-08-05 13:12 ` Andrew Lunn
0 siblings, 1 reply; 4+ messages in thread
From: Chenguang Zhao @ 2026-08-05 6:14 UTC (permalink / raw)
To: marcin.s.wojtas, andrew+netdev, davem, edumazet, kuba, pabeni
Cc: netdev, chenguang.zhao, Chenguang Zhao
From: Chenguang Zhao <zhaochenguang@kylinos.cn>
gen_pool_free() expects the virtual address returned by
gen_pool_dma_alloc(), not the physical address. Passing phys can miss
the chunk and trigger BUG() on remove or probe rollback.
Fixes: dc35a10f68d3 ("net: mvneta: bm: add support for hardware buffer management")
Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn>
---
drivers/net/ethernet/marvell/mvneta_bm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/mvneta_bm.c b/drivers/net/ethernet/marvell/mvneta_bm.c
index 6bb380494919..2a114a69a0ab 100644
--- a/drivers/net/ethernet/marvell/mvneta_bm.c
+++ b/drivers/net/ethernet/marvell/mvneta_bm.c
@@ -389,7 +389,7 @@ static int mvneta_bm_get_sram(struct device_node *dn,
static void mvneta_bm_put_sram(struct mvneta_bm *priv)
{
- gen_pool_free(priv->bppi_pool, priv->bppi_phys_addr,
+ gen_pool_free(priv->bppi_pool, (unsigned long)priv->bppi_virt_addr,
MVNETA_BM_BPPI_SIZE);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net] net: mvneta_bm: fix gen_pool_free address for BPPI 2026-08-05 6:14 [PATCH net] net: mvneta_bm: fix gen_pool_free address for BPPI Chenguang Zhao @ 2026-08-05 13:12 ` Andrew Lunn 2026-08-06 8:13 ` Chenguang Zhao 0 siblings, 1 reply; 4+ messages in thread From: Andrew Lunn @ 2026-08-05 13:12 UTC (permalink / raw) To: Chenguang Zhao Cc: marcin.s.wojtas, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, Chenguang Zhao On Wed, Aug 05, 2026 at 02:14:38PM +0800, Chenguang Zhao wrote: > From: Chenguang Zhao <zhaochenguang@kylinos.cn> > > gen_pool_free() expects the virtual address returned by > gen_pool_dma_alloc(), not the physical address. Passing phys can miss > the chunk and trigger BUG() on remove or probe rollback. > > Fixes: dc35a10f68d3 ("net: mvneta: bm: add support for hardware buffer management") > Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn> > --- > drivers/net/ethernet/marvell/mvneta_bm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/mvneta_bm.c b/drivers/net/ethernet/marvell/mvneta_bm.c > index 6bb380494919..2a114a69a0ab 100644 > --- a/drivers/net/ethernet/marvell/mvneta_bm.c > +++ b/drivers/net/ethernet/marvell/mvneta_bm.c > @@ -389,7 +389,7 @@ static int mvneta_bm_get_sram(struct device_node *dn, > > static void mvneta_bm_put_sram(struct mvneta_bm *priv) > { > - gen_pool_free(priv->bppi_pool, priv->bppi_phys_addr, > + gen_pool_free(priv->bppi_pool, (unsigned long)priv->bppi_virt_addr, The cast is ugly, but it also seems correct. However, can the API be improved. If the intention is gen_pool_free(priv->bppi_pool, gen_pool_dma_alloc(priv->bppi_pool, MVNETA_BM_BPPI_SIZE, &priv->bppi_phys_addr), MVNETA_BM_BPPI_SIZE); maybe gen_pool_free() should be changed to take a void *? Andrew ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: mvneta_bm: fix gen_pool_free address for BPPI 2026-08-05 13:12 ` Andrew Lunn @ 2026-08-06 8:13 ` Chenguang Zhao 2026-08-06 16:16 ` Andrew Lunn 0 siblings, 1 reply; 4+ messages in thread From: Chenguang Zhao @ 2026-08-06 8:13 UTC (permalink / raw) To: Andrew Lunn Cc: marcin.s.wojtas, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, Chenguang Zhao 在 2026/8/5 21:12, Andrew Lunn 写道: > On Wed, Aug 05, 2026 at 02:14:38PM +0800, Chenguang Zhao wrote: >> From: Chenguang Zhao <zhaochenguang@kylinos.cn> >> >> gen_pool_free() expects the virtual address returned by >> gen_pool_dma_alloc(), not the physical address. Passing phys can miss >> the chunk and trigger BUG() on remove or probe rollback. >> >> Fixes: dc35a10f68d3 ("net: mvneta: bm: add support for hardware buffer management") >> Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn> >> --- >> drivers/net/ethernet/marvell/mvneta_bm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/marvell/mvneta_bm.c b/drivers/net/ethernet/marvell/mvneta_bm.c >> index 6bb380494919..2a114a69a0ab 100644 >> --- a/drivers/net/ethernet/marvell/mvneta_bm.c >> +++ b/drivers/net/ethernet/marvell/mvneta_bm.c >> @@ -389,7 +389,7 @@ static int mvneta_bm_get_sram(struct device_node *dn, >> >> static void mvneta_bm_put_sram(struct mvneta_bm *priv) >> { >> - gen_pool_free(priv->bppi_pool, priv->bppi_phys_addr, >> + gen_pool_free(priv->bppi_pool, (unsigned long)priv->bppi_virt_addr, > The cast is ugly, but it also seems correct. However, can the API be > improved. If the intention is > > gen_pool_free(priv->bppi_pool, > gen_pool_dma_alloc(priv->bppi_pool, > MVNETA_BM_BPPI_SIZE, > &priv->bppi_phys_addr), > MVNETA_BM_BPPI_SIZE); > > maybe gen_pool_free() should be changed to take a void *? > > Andrew Thanks for the suggestion. The cast is indeed a bit ugly, but several other drivers free memory from gen_pool_dma_alloc() in the same way today. Changing gen_pool_free() itself to take a void * would touch quite a few call sites, including ones that use genalloc for non-pointer cookies, so that may be a larger change than we want for this fix. If we do want to improve the API, perhaps a small helper would be enough, for example: static inline void gen_pool_dma_free(struct gen_pool *pool, void *vaddr, size_t size) { gen_pool_free(pool, (unsigned long)vaddr, size); } Does that sound reasonable to you? I'm happy to keep this bugfix as-is for now, or follow up with such a helper if preferred. Chenguang ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: mvneta_bm: fix gen_pool_free address for BPPI 2026-08-06 8:13 ` Chenguang Zhao @ 2026-08-06 16:16 ` Andrew Lunn 0 siblings, 0 replies; 4+ messages in thread From: Andrew Lunn @ 2026-08-06 16:16 UTC (permalink / raw) To: Chenguang Zhao Cc: marcin.s.wojtas, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, Chenguang Zhao > > The cast is ugly, but it also seems correct. However, can the API be > > improved. If the intention is > > > > gen_pool_free(priv->bppi_pool, > > gen_pool_dma_alloc(priv->bppi_pool, > > MVNETA_BM_BPPI_SIZE, > > &priv->bppi_phys_addr), > > MVNETA_BM_BPPI_SIZE); > > > > maybe gen_pool_free() should be changed to take a void *? > > > > Andrew > Thanks for the suggestion. > > The cast is indeed a bit ugly, but several other drivers free memory from > gen_pool_dma_alloc() in the same way today. Flip it around. Are there many calls which don't have the cast? > Changing gen_pool_free() > itself to take a void * would touch quite a few call sites, including > ones that use genalloc for non-pointer cookies, so that may be a larger > change than we want for this fix. I agree it is more than just the fix here. But this fix is adding yet another instance where maybe bad design is forcing a cast. It should also be quite a mechanical change. Can Coccinelle do the change needed? Andrew ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-06 16:16 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-05 6:14 [PATCH net] net: mvneta_bm: fix gen_pool_free address for BPPI Chenguang Zhao 2026-08-05 13:12 ` Andrew Lunn 2026-08-06 8:13 ` Chenguang Zhao 2026-08-06 16:16 ` Andrew Lunn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox