DMA memory, split_page, BUG_ON(PageCompound()), sound

DMA memory, split_page, BUG_ON(PageCompound()), sound

[Marc Singer]

I'm investigating why I am triggering a BUG_ON in split_page() when I
use the sound subsystems dma memory allocation aide.

The crux of the problem appears to be that snd_malloc_dev_pages()
passes __GFP_COMP into dma_alloc_coherent().  On the ARM and several
other architectures, the dma allocation code calls split_page () with
pages allocated with this flag which, in turn, triggers the BUG_ON()
check for the CompoundPage flag.

So, the questions are these: Who is doing the wrong thing?  Should the
snd_malloc_dev_pages() call drop the __GFP_COMP flag?  Should
split_page() allow the page to be compound?  Should __GFP_COMP be 0 on
ARM and other architectures that don't support compound pages?

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Nick Piggin]

Marc Singer wrote:
> I'm investigating why I am triggering a BUG_ON in split_page() when I
> use the sound subsystems dma memory allocation aide.
> 
> The crux of the problem appears to be that snd_malloc_dev_pages()
> passes __GFP_COMP into dma_alloc_coherent().  On the ARM and several
> other architectures, the dma allocation code calls split_page () with
> pages allocated with this flag which, in turn, triggers the BUG_ON()
> check for the CompoundPage flag.
> 
> So, the questions are these: Who is doing the wrong thing?  Should the
> snd_malloc_dev_pages() call drop the __GFP_COMP flag?  Should
> split_page() allow the page to be compound?  Should __GFP_COMP be 0 on
> ARM and other architectures that don't support compound pages?

I personally never liked the explicit __GFP_COMP going in everywhere,
and would have much preferred a GFP_USERMAP, which the architecture /
allocator could satisfy as they liked.

As a hack, you can make arm's dma_alloc_coherent() drop __GFP_COMP,
which should work.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Marc Singer]

On Sun, Jul 09, 2006 at 01:26:06PM +1000, Nick Piggin wrote:
> Marc Singer wrote:
> >I'm investigating why I am triggering a BUG_ON in split_page() when I
> >use the sound subsystems dma memory allocation aide.
> >
> >The crux of the problem appears to be that snd_malloc_dev_pages()
> >passes __GFP_COMP into dma_alloc_coherent().  On the ARM and several
> >other architectures, the dma allocation code calls split_page () with
> >pages allocated with this flag which, in turn, triggers the BUG_ON()
> >check for the CompoundPage flag.
> >
> >So, the questions are these: Who is doing the wrong thing?  Should the
> >snd_malloc_dev_pages() call drop the __GFP_COMP flag?  Should
> >split_page() allow the page to be compound?  Should __GFP_COMP be 0 on
> >ARM and other architectures that don't support compound pages?
> 
> I personally never liked the explicit __GFP_COMP going in everywhere,
> and would have much preferred a GFP_USERMAP, which the architecture /
> allocator could satisfy as they liked.

Thus, the __GFP_COMP bit would be part of another flags such that it
is set on x86 (or any other architecture that supports it) and clear
on those that don't.

> As a hack, you can make arm's dma_alloc_coherent() drop __GFP_COMP,
> which should work.

There are many architectures that have this problem, so I suspect that
such a patch would be unappreciated.

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Nick Piggin]

Marc Singer wrote:
> On Sun, Jul 09, 2006 at 01:26:06PM +1000, Nick Piggin wrote:
> 
>>Marc Singer wrote:
>>
>>>I'm investigating why I am triggering a BUG_ON in split_page() when I
>>>use the sound subsystems dma memory allocation aide.
>>>
>>>The crux of the problem appears to be that snd_malloc_dev_pages()
>>>passes __GFP_COMP into dma_alloc_coherent().  On the ARM and several
>>>other architectures, the dma allocation code calls split_page () with
>>>pages allocated with this flag which, in turn, triggers the BUG_ON()
>>>check for the CompoundPage flag.
>>>
>>>So, the questions are these: Who is doing the wrong thing?  Should the
>>>snd_malloc_dev_pages() call drop the __GFP_COMP flag?  Should
>>>split_page() allow the page to be compound?  Should __GFP_COMP be 0 on
>>>ARM and other architectures that don't support compound pages?
>>
>>I personally never liked the explicit __GFP_COMP going in everywhere,
>>and would have much preferred a GFP_USERMAP, which the architecture /
>>allocator could satisfy as they liked.
> 
> 
> Thus, the __GFP_COMP bit would be part of another flags such that it
> is set on x86 (or any other architecture that supports it) and clear
> on those that don't.

I guess you could do it a number of ways. Maybe having GFP_USERMAP
set __GFP_USERMAP|__GFP_COMP, and the arm dma memory allocator can
strip the __GFP_COMP.

If you get an explicit __GFP_COMP passed down, the allocator doesn't
know whether that was because they want a user mappable area, or
really want a compound page (in which case, stripping __GFP_COMP is
the wrong thing to do).

> 
> 
>>As a hack, you can make arm's dma_alloc_coherent() drop __GFP_COMP,
>>which should work.
> 
> 
> There are many architectures that have this problem, so I suspect that
> such a patch would be unappreciated.
> 

Hacks usually are ;) It would get you working though.

If you want to write a patch that would be appreciated, that would be
appreciated.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Russell King]

On Mon, Jul 10, 2006 at 04:59:48PM +1000, Nick Piggin wrote:
> I guess you could do it a number of ways. Maybe having GFP_USERMAP
> set __GFP_USERMAP|__GFP_COMP, and the arm dma memory allocator can
> strip the __GFP_COMP.
> 
> If you get an explicit __GFP_COMP passed down, the allocator doesn't
> know whether that was because they want a user mappable area, or
> really want a compound page (in which case, stripping __GFP_COMP is
> the wrong thing to do).

So I'll mask off __GFP_COMP for the time being in the ARM dma allocator
with a note to this effect?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Nick Piggin]

Russell King wrote:
> On Mon, Jul 10, 2006 at 04:59:48PM +1000, Nick Piggin wrote:
> 
>>I guess you could do it a number of ways. Maybe having GFP_USERMAP
>>set __GFP_USERMAP|__GFP_COMP, and the arm dma memory allocator can
>>strip the __GFP_COMP.
>>
>>If you get an explicit __GFP_COMP passed down, the allocator doesn't
>>know whether that was because they want a user mappable area, or
>>really want a compound page (in which case, stripping __GFP_COMP is
>>the wrong thing to do).
> 
> 
> So I'll mask off __GFP_COMP for the time being in the ARM dma allocator
> with a note to this effect?

I believe that should do the trick, yes (AFAIK, nobody yet is
explicitly relying on a compound page from the dma allocator).

Marc can hopefully confim the fix.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Marc Singer]

On Tue, Jul 11, 2006 at 03:34:11AM +1000, Nick Piggin wrote:
> Russell King wrote:
> >On Mon, Jul 10, 2006 at 04:59:48PM +1000, Nick Piggin wrote:
> >
> >>I guess you could do it a number of ways. Maybe having GFP_USERMAP
> >>set __GFP_USERMAP|__GFP_COMP, and the arm dma memory allocator can
> >>strip the __GFP_COMP.
> >>
> >>If you get an explicit __GFP_COMP passed down, the allocator doesn't
> >>know whether that was because they want a user mappable area, or
> >>really want a compound page (in which case, stripping __GFP_COMP is
> >>the wrong thing to do).
> >
> >
> >So I'll mask off __GFP_COMP for the time being in the ARM dma allocator
> >with a note to this effect?
> 
> I believe that should do the trick, yes (AFAIK, nobody yet is
> explicitly relying on a compound page from the dma allocator).
> 
> Marc can hopefully confim the fix.

I'll verify that that works, yes.

> 
> -- 
> SUSE Labs, Novell Inc.
> Send instant messages to your online friends http://au.messenger.yahoo.com 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Marc Singer]

On Tue, Jul 11, 2006 at 03:34:11AM +1000, Nick Piggin wrote:
> >So I'll mask off __GFP_COMP for the time being in the ARM dma allocator
> >with a note to this effect?
> 
> I believe that should do the trick, yes (AFAIK, nobody yet is
> explicitly relying on a compound page from the dma allocator).
> 
> Marc can hopefully confim the fix.

The patch, attached, works in that there is no BUG_ON() trap.

>From c13ac90b1dd6e5fab63182ce323673cdffb0b55f Mon Sep 17 00:00:00 2001
Date: Mon, 10 Jul 2006 19:38:47 -0700
Subject: [PATCH] Drop __GFP_COMP for DMA memory allocations
---
 arch/arm/mm/consistent.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

Hack for incompatible __GFP_COMP flag making its way into the ARM DMA
consistent memory allocator.

Signed-off-by: Marc Singer <elf@buici.com>

diff --git a/arch/arm/mm/consistent.c b/arch/arm/mm/consistent.c
index 50e6b6b..7b7cc4e 100644
--- a/arch/arm/mm/consistent.c
+++ b/arch/arm/mm/consistent.c
@@ -153,6 +153,13 @@ __dma_alloc(struct device *dev, size_t s
 	unsigned long order;
 	u64 mask = ISA_DMA_THRESHOLD, limit;
 
+	/* Following is a work-around (a.k.a. hack) to prevent pages
+	 * with __GFP_COMP being passed to split_page() which cannot
+	 * handle them.  The real problem is that this flag probably
+	 * should be 0 on ARM as it is not supported on this
+	 * platform--see CONFIG_HUGETLB_PAGE. */
+	gfp &= ~(__GFP_COMP);
+
 	if (!consistent_pte[0]) {
 		printk(KERN_ERR "%s: not initialised\n", __func__);
 		dump_stack();
-- 
1.4.0

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Russell King]

On Tue, Jul 11, 2006 at 03:34:11AM +1000, Nick Piggin wrote:
> Russell King wrote:
> >On Mon, Jul 10, 2006 at 04:59:48PM +1000, Nick Piggin wrote:
> >
> >>I guess you could do it a number of ways. Maybe having GFP_USERMAP
> >>set __GFP_USERMAP|__GFP_COMP, and the arm dma memory allocator can
> >>strip the __GFP_COMP.
> >>
> >>If you get an explicit __GFP_COMP passed down, the allocator doesn't
> >>know whether that was because they want a user mappable area, or
> >>really want a compound page (in which case, stripping __GFP_COMP is
> >>the wrong thing to do).
> >
> >
> >So I'll mask off __GFP_COMP for the time being in the ARM dma allocator
> >with a note to this effect?
> 
> I believe that should do the trick, yes (AFAIK, nobody yet is
> explicitly relying on a compound page from the dma allocator).

In which case should ALSA be passing __GFP_COMP to the dma allocator ?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Takashi Iwai]

At Wed, 12 Jul 2006 11:32:41 +0100,
Russell King wrote:
> 
> On Tue, Jul 11, 2006 at 03:34:11AM +1000, Nick Piggin wrote:
> > Russell King wrote:
> > >On Mon, Jul 10, 2006 at 04:59:48PM +1000, Nick Piggin wrote:
> > >
> > >>I guess you could do it a number of ways. Maybe having GFP_USERMAP
> > >>set __GFP_USERMAP|__GFP_COMP, and the arm dma memory allocator can
> > >>strip the __GFP_COMP.
> > >>
> > >>If you get an explicit __GFP_COMP passed down, the allocator doesn't
> > >>know whether that was because they want a user mappable area, or
> > >>really want a compound page (in which case, stripping __GFP_COMP is
> > >>the wrong thing to do).
> > >
> > >
> > >So I'll mask off __GFP_COMP for the time being in the ARM dma allocator
> > >with a note to this effect?
> > 
> > I believe that should do the trick, yes (AFAIK, nobody yet is
> > explicitly relying on a compound page from the dma allocator).
> 
> In which case should ALSA be passing __GFP_COMP to the dma allocator ?

I would be willing to remove __GFP_COMP if it's not needed :)
Passing this flag is really confusing.  The driver doesn't use the
compound pages at all but it was added just to enable mmap support.
So, Nick's proposal appears reasonable to me.


Takashi

Re: DMA memory, split_page, BUG_ON(PageCompound()), sound

[Nick Piggin]

Takashi Iwai wrote:
> At Wed, 12 Jul 2006 11:32:41 +0100,

>>In which case should ALSA be passing __GFP_COMP to the dma allocator ?

Yes, sorry I didn't answer your question: ALSA basically wants to be
able to have that memory mmapable by userspace programs, and it doesn't
care whether this is via compound pages or split pages.

> I would be willing to remove __GFP_COMP if it's not needed :)
> Passing this flag is really confusing.  The driver doesn't use the
> compound pages at all but it was added just to enable mmap support.
> So, Nick's proposal appears reasonable to me.

I don't think it would be hard. Just need a bit of thinking about how
to go about composing some sort of GFP_USERMAP that can easily be
handled by all allocators, without costing performance or being too
intrusive.

I suspect I won't have time to wade into such an exercise until after
OLS. But I could give a quick review for anyone who does ;)

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com