* [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