Re: [PATCH] MIPS: Alchemy: fix dbdma ring destruction memory debugcheck.

[David Daney <ddaney@caviumnetworks.com>]

Manuel Lauss wrote:
> DBDMA descriptors need to be located at 32-byte aligned addresses;
> however kmalloc rarely delivers such addresses.  The dbdma code
> works around that by allocating 63 bytes and re-aligning the
> descriptor base afterwards.  Hoewever when freeing memory it does
> not account for this adjustment and trips the kfree debugcheck:
> 

Correct me if I am wrong, but don't kmalloc et al. return blocks aligned 
   boundaries of the size rounded up the the next power of two?  So if 
you need 32-byte aligned addresses, just use a size value of 32 or 
greater.  You wouldn't have to add 63 and do masking and remember the 
membase value as you do in the patch.

David Daney



> Kernel bug detected[#1]:
> [...]
> Call Trace:
> [<80186010>] cache_free_debugcheck+0x284/0x318
> [<801869d8>] kfree+0xe8/0x2a0
> [<8010b31c>] au1xxx_dbdma_chan_free+0x2c/0x7c
> [<80388dc8>] au1x_pcm_dbdma_free+0x34/0x4c
> [<80388fa8>] au1xpsc_pcm_close+0x28/0x38
> [<80383cb8>] soc_codec_close+0x14c/0x1cc
> [<8036dbb4>] snd_pcm_release_substream+0x60/0xac
> [<8036dc40>] snd_pcm_release+0x40/0xa0
> [<8018c7a8>] __fput+0x11c/0x228
> [<80188f60>] filp_close+0x7c/0x98
> [<80189018>] sys_close+0x9c/0xe4
> [<801022a0>] stack_done+0x20/0x3c
> 
> Fix this by recording the address delivered by kmalloc() and using
> it as parameter to kfree().
> 
> Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
> ---
>  arch/mips/alchemy/common/dbdma.c                 |    7 +++++--
>  arch/mips/include/asm/mach-au1x00/au1xxx_dbdma.h |    1 +
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/alchemy/common/dbdma.c b/arch/mips/alchemy/common/dbdma.c
> index 40071bd..3b2ccc0 100644
> --- a/arch/mips/alchemy/common/dbdma.c
> +++ b/arch/mips/alchemy/common/dbdma.c
> @@ -411,8 +411,11 @@ u32 au1xxx_dbdma_ring_alloc(u32 chanid, int entries)
>  		if (desc_base == 0)
>  			return 0;
>  
> +		ctp->cdb_membase = desc_base;
>  		desc_base = ALIGN_ADDR(desc_base, sizeof(au1x_ddma_desc_t));
> -	}
> +	} else
> +		ctp->cdb_membase = desc_base;
> +
>  	dp = (au1x_ddma_desc_t *)desc_base;
>  
>  	/* Keep track of the base descriptor. */
> @@ -829,7 +832,7 @@ void au1xxx_dbdma_chan_free(u32 chanid)
>  
>  	au1xxx_dbdma_stop(chanid);
>  
> -	kfree((void *)ctp->chan_desc_base);
> +	kfree((void *)ctp->cdb_membase);
>  
>  	stp->dev_flags &= ~DEV_FLAGS_INUSE;
>  	dtp->dev_flags &= ~DEV_FLAGS_INUSE;
> diff --git a/arch/mips/include/asm/mach-au1x00/au1xxx_dbdma.h b/arch/mips/include/asm/mach-au1x00/au1xxx_dbdma.h
> index c098b45..8c6b110 100644
> --- a/arch/mips/include/asm/mach-au1x00/au1xxx_dbdma.h
> +++ b/arch/mips/include/asm/mach-au1x00/au1xxx_dbdma.h
> @@ -305,6 +305,7 @@ typedef struct dbdma_chan_config {
>  	dbdev_tab_t		*chan_dest;
>  	au1x_dma_chan_t		*chan_ptr;
>  	au1x_ddma_desc_t	*chan_desc_base;
> +	u32			cdb_membase; /* kmalloc base of above */
>  	au1x_ddma_desc_t	*get_ptr, *put_ptr, *cur_ptr;
>  	void			*chan_callparam;
>  	void			(*chan_callback)(int, void *);