Re: [RFC,PATCH 2/2] dmapool: Honor GFP_* flags.

[Roger Oksanen <roger.oksanen@cs.helsinki.fi>]

On Wednesday 02 December 2009 22:00:56 Christoph Lameter wrote:
> On Wed, 2 Dec 2009, Roger Oksanen wrote:
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/mm/dmapool.c b/mm/dmapool.c
> > index 2fdd7a1..e270f7f 100644
> > --- a/mm/dmapool.c
> > +++ b/mm/dmapool.c
> > @@ -312,6 +312,8 @@
> >  	void *retval;
> >  	int tries = 0;
> >  	const gfp_t can_wait = mem_flags & __GFP_WAIT;
> > +	/* dma_pool_alloc uses its own wait logic */
> > +	mem_flags &= ~__GFP_WAIT;
> 
> Why mask the wait flag? If you can call the page allocator with __GFP_WAIT
> then you dont have to loop.

That would fundamentally change how the pool allocator works. Currently it 
waits on its own wait queue for returned memory from dma_pool_free(..). 
Waiting in the page allocator won't allow it to claim memory returned there. 
Now if it would be possible (is it?), it should sit on both wait queues.


> > -	page = pool_alloc_page(pool, GFP_ATOMIC | (can_wait && tries % 10
> > +	page = pool_alloc_page(pool, mem_flags | (can_wait && tries % 10
> >  						  ? __GFP_NOWARN : 0));
> 
> You are now uselessly calling the page allocator with __GFP_WAIT cleared
> although the context allows you to wait.
> 
> Just pass through the mem_flags? Rename them gfp_flags for consistencies
> sake?

Killing off the pools own wait logic is indeed possible. The question is 
though, is it desirable? Is allocating dma memory on some arches so expensive 
that its crucial to try to avoid any extra allocations when pooled memory may 
be freed up in a few moments?

best regards,
-- 
Roger Oksanen <roger.oksanen@cs.helsinki.fi>
http://www.cs.helsinki.fi/u/raoksane
+358 50 355 1990

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>