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

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

[Manuel Lauss]

DBDMA descriptors need to be located at 32-byte aligned addresses;
however kmalloc in conjunction with the SLAB allocator and 
CONFIG_DEBUG_SLUB enabled doesn't deliver any.  The dbdma code works
around that by allocating a larger area and realigning the start
address within it.

When freeing a channel however this adjustment is not taken into
account which results in an oops:

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().

This fix is only necessary with the SLAB allocator and CONFIG_DEBUG_SLAB
enabled;  non-debug SLAB, SLUB do return nicely aligned addresses,
debug-enabled SLUB currently panics early in the boot process.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
v2: slightly improved the patch rationale, thanks to David Daney.

 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 *);
-- 
1.6.6

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

[Ralf Baechle]

On Tue, Jan 26, 2010 at 08:39:33PM +0100, Manuel Lauss wrote:

> DBDMA descriptors need to be located at 32-byte aligned addresses;
> however kmalloc in conjunction with the SLAB allocator and 
> CONFIG_DEBUG_SLUB enabled doesn't deliver any.  The dbdma code works
> around that by allocating a larger area and realigning the start
> address within it.
> 
> When freeing a channel however this adjustment is not taken into
> account which results in an oops:
> 
> 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().
> 
> This fix is only necessary with the SLAB allocator and CONFIG_DEBUG_SLAB
> enabled;  non-debug SLAB, SLUB do return nicely aligned addresses,
> debug-enabled SLUB currently panics early in the boot process.

Queued for 2.6.34 - should this also go into 2.6.33?

Have you considered increasing the value ARCH_KMALLOC_MINALIGN which
defaults to 8?  Or your own slab cache of suitable alignment?  The latter
is more something for frequent allocations.

  Ralf

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

[Manuel Lauss]

>> This fix is only necessary with the SLAB allocator and CONFIG_DEBUG_SLAB
>> enabled;  non-debug SLAB, SLUB do return nicely aligned addresses,
>> debug-enabled SLUB currently panics early in the boot process.
>
> Queued for 2.6.34 - should this also go into 2.6.33?

2.6.33 and .32 at least.  That code has been in there since the dawn of 2.6;
the fact that nobody has tripped over this so far means that noone build kernels
with slab debugging (I tripped over this while looking for something else) or
the slab allocator behaviour changed recently.


> Have you considered increasing the value ARCH_KMALLOC_MINALIGN which
> defaults to 8?  Or your own slab cache of suitable alignment?  The latter
> is more something for frequent allocations.

I have to admit I know nothing about Linux' memory management.  Are slab
caches not affected by the what I presume are "guard bytes" inserted into
the memory areas when slab debug is on?

Manuel

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

[Manuel Lauss]

>> This fix is only necessary with the SLAB allocator and CONFIG_DEBUG_SLAB
>> enabled;  non-debug SLAB, SLUB do return nicely aligned addresses,
>> debug-enabled SLUB currently panics early in the boot process.
>
> Queued for 2.6.34 - should this also go into 2.6.33?

2.6.33 and .32 at least.  That code has been in there since the dawn of 2.6;
the fact that nobody has tripped over this so far means that noone build kernels
with slab debugging (I tripped over this while looking for something else) or
the slab allocator behaviour changed recently.


> Have you considered increasing the value ARCH_KMALLOC_MINALIGN which
> defaults to 8?  Or your own slab cache of suitable alignment?  The latter
> is more something for frequent allocations.

I have to admit I know nothing about Linux' memory management.  Are slab
caches not affected by the what I presume are "guard bytes" inserted into
the memory areas when slab debug is on?

Manuel

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

[Ralf Baechle]

On Thu, Jan 28, 2010 at 09:23:56AM +0100, Manuel Lauss wrote:

> >> This fix is only necessary with the SLAB allocator and CONFIG_DEBUG_SLAB
> >> enabled;  non-debug SLAB, SLUB do return nicely aligned addresses,
> >> debug-enabled SLUB currently panics early in the boot process.
> >
> > Queued for 2.6.34 - should this also go into 2.6.33?
> 
> 2.6.33 and .32 at least.  That code has been in there since the dawn of 2.6;
> the fact that nobody has tripped over this so far means that noone build kernels
> with slab debugging (I tripped over this while looking for something else) or
> the slab allocator behaviour changed recently.

The behaviour of the slab has been like this as long as I can remember.
Cherrypicking into the -stable branches as I'm writing this.

> > Have you considered increasing the value ARCH_KMALLOC_MINALIGN which
> > defaults to 8?  Or your own slab cache of suitable alignment?  The latter
> > is more something for frequent allocations.
> 
> I have to admit I know nothing about Linux' memory management.  Are slab
> caches not affected by the what I presume are "guard bytes" inserted into
> the memory areas when slab debug is on?

When creating a slab cache an alignment for objects allocated from this
cache can be specified.

  Ralf