Netdev List
 help / color / mirror / Atom feed
* [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