Re: [PATCH net] net: mvneta_bm: fix gen_pool_free address for BPPI

在 2026/8/5 21:12, Andrew Lunn 写道:
> On Wed, Aug 05, 2026 at 02:14:38PM +0800, Chenguang Zhao wrote:
>> From: Chenguang Zhao <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>>  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