Re: [PATCH] 1/3 Implement generic device DMA mapping support

[Takashi Iwai <tiwai@suse.de>]

At Mon, 1 Mar 2004 18:22:24 +0000,
Russell King wrote:
> 
> On Mon, Mar 01, 2004 at 06:51:56PM +0100, Takashi Iwai wrote:
> > a small concern about GFP_KERNEL is that i experienced the stall when
> > the kernel tried to allocate large continuous pages with GFP_KERNEL,
> > e.g. modprobe stops infinitely in the module init phase (and you
> > cannot even interrupt that process).
> > 
> > does dma_alloc_coherent(GFP_KERNEL) with big pages work without stall?
> 
> It depends where the stall was coming from.  Do you have any further
> details?

first of alll, i have to mention that it happend in the time of 2.4
kernels.  i've not tested with 2.6 kernels at all.

a typical case was es1968 driver, which allocates the all buffer pages
at the initialization.  when __get_free_pages() for 256MB with
GFP_KERNEL is called, it goes to sleep and never gets back.  since
the context is uninterruptible, modprobe stucks and doesn't accept
SIGKILL.

anyway, i need to this behavior again with the recent kernel.


> Also, on a similar note, I hope someone noticed one of the comments I
> added in the second patch:
> 
> + *
> + * Really, we want to move this type of thing into dma_alloc_coherent()
> + * so dma_mask doesn't have to be messed with.
> 
> (referring to the hack towards the top of memalloc.c.)
> 
> This is something which really needs solving more cleanly.

yep!

>  What I
> think you're trying to do is to allocate pages for devices whose
> DMA mask indicates (eg) 28 bit addressing is possible, but without
> the "ISA DMA" (<16MB) restriction?  Could you confirm this?

yes, that's it.

> If this is correct, I suspect it may be something which is not
> limited to sound devices, and as such should probably be covered
> by the generic code (iow, dma_alloc_coherent.)  However, that's
> going to require discussion with other several people.

i hope it, too.  for example, this kind of large buffer allocation
might be needed for video devices, too.


> Currently, x86 dma_alloc_coherent() is:
> 
>         if (dev == NULL || (*dev->dma_mask < 0xffffffff))
>                 gfp |= GFP_DMA;
>         ret = (void *)__get_free_pages(gfp, get_order(size));
>  
>         if (ret != NULL) {
>                 memset(ret, 0, size);
>                 *dma_handle = virt_to_phys(ret);
>         }
>         return ret;
> 
> I'm thinking of something more like:
> 
> 	order = get_order(size);
> 
> 	mask = 16*1024*1024-1;
> 	if (dev)
> 		mask = dev->dma_mask;
> 
> 	while (1) {
> 		ret = (void *)__get_free_pages(gfp, order);
> 
> 		if (ret == NULL || gfp & GFP_DMA)
> 			break;
> 
> 		handle = virt_to_phys(ret);
> 		if ((handle & ~mask) == 0) {
> 			memset(ret, 0, size);
> 			*dma_handle = handle;
> 			break;
> 		}
> 
> 		free_pages((unsigned long)ret, order);
> 		gfp |= GFP_DMA;
> 	}
> 
> 	return ret;
> 
> Does that look reasonable?  If so, I'll float the idea around x86
> people.

yes, it looks fine.  thanks!


Takashi


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click