Re: [PATCH] mm: dmapool: use provided gfp flags for all dma_alloc_coherent()calls

[Soeren Moch]

On 11.11.2012 18:22, Andrew Lunn wrote:
> On Thu, Nov 08, 2012 at 07:38:57AM +0100, Marek Szyprowski wrote:
>> dmapool always calls dma_alloc_coherent() with GFP_ATOMIC flag, 
regardless
>> the flags provided by the caller. This causes excessive pruning of
>> emergency memory pools without any good reason. This patch changes 
the code
>> to correctly use gfp flags provided by the dmapool caller. This should
>> solve the dmapool usage on ARM architecture, where GFP_ATOMIC DMA
>> allocations can be served only from the special, very limited memory 
pool.
>>
>> Reported-by: Soren Moch <smoch@xxxxxx>
Please use
Reported-by: Soeren Moch <smoch@xxxxxx>

>> Reported-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>
> Tested-by: Andrew Lunn <andrew@xxxxxxx>
>
> I tested this on a Kirkwood QNAP after removing the call to
> init_dma_coherent_pool_size().
>
>     Andrew

Tested-by: Soeren Moch <smoch@xxxxxx>

Now I had a chance to test this patch on my Kirkwood guruplug
system with linux-3.6.6 . It is running much better now, but with the
original 256K coherent pool size I still see errors after several hours
of runtime:

Nov 12 09:42:32 guru kernel: ERROR: 256 KiB atomic DMA coherent pool is 
too small!
Nov 12 09:42:32 guru kernel: Please increase it with coherent_pool= 
kernel parameter!

  Soeren

>> ---
>>  mm/dmapool.c |   27 +++++++--------------------
>>  1 file changed, 7 insertions(+), 20 deletions(-)
>>
>> diff --git a/mm/dmapool.c b/mm/dmapool.c
>> index c5ab33b..86de9b2 100644
>> --- a/mm/dmapool.c
>> +++ b/mm/dmapool.c
>> @@ -62,8 +62,6 @@ struct dma_page {        /* cacheable header for 
'allocation' bytes */
>>      unsigned int offset;
>>  };
>>
>> -#define    POOL_TIMEOUT_JIFFIES    ((100 /* msec */ * HZ) / 1000)
>> -
>>  static DEFINE_MUTEX(pools_lock);
>>
>>  static ssize_t
>> @@ -227,7 +225,6 @@ static struct dma_page *pool_alloc_page(struct 
dma_pool *pool, gfp_t mem_flags)
>>          memset(page->vaddr, POOL_POISON_FREED, pool->allocation);
>>  #endif
>>          pool_initialise_page(pool, page);
>> -        list_add(&page->page_list, &pool->page_list);
>>          page->in_use = 0;
>>          page->offset = 0;
>>      } else {
>> @@ -315,30 +312,21 @@ void *dma_pool_alloc(struct dma_pool *pool, 
gfp_t mem_flags,
>>      might_sleep_if(mem_flags & __GFP_WAIT);
>>
>>      spin_lock_irqsave(&pool->lock, flags);
>> - restart:
>>      list_for_each_entry(page, &pool->page_list, page_list) {
>>          if (page->offset < pool->allocation)
>>              goto ready;
>>      }
>> -    page = pool_alloc_page(pool, GFP_ATOMIC);
>> -    if (!page) {
>> -        if (mem_flags & __GFP_WAIT) {
>> -            DECLARE_WAITQUEUE(wait, current);
>>
>> -            __set_current_state(TASK_UNINTERRUPTIBLE);
>> -            __add_wait_queue(&pool->waitq, &wait);
>> -            spin_unlock_irqrestore(&pool->lock, flags);
>> +    /* pool_alloc_page() might sleep, so temporarily drop 
&pool->lock */
>> +    spin_unlock_irqrestore(&pool->lock, flags);
>>
>> -            schedule_timeout(POOL_TIMEOUT_JIFFIES);
>> +    page = pool_alloc_page(pool, mem_flags);
>> +    if (!page)
>> +        return NULL;
>>
>> -            spin_lock_irqsave(&pool->lock, flags);
>> -            __remove_wait_queue(&pool->waitq, &wait);
>> -            goto restart;
>> -        }
>> -        retval = NULL;
>> -        goto done;
>> -    }
>> +    spin_lock_irqsave(&pool->lock, flags);
>>
>> +    list_add(&page->page_list, &pool->page_list);
>>   ready:
>>      page->in_use++;
>>      offset = page->offset;
>> @@ -348,7 +336,6 @@ void *dma_pool_alloc(struct dma_pool *pool, 
gfp_t mem_flags,
>>  #ifdef    DMAPOOL_DEBUG
>>      memset(retval, POOL_POISON_ALLOCATED, pool->size);
>>  #endif
>> - done:
>>      spin_unlock_irqrestore(&pool->lock, flags);
>>      return retval;
>>  }
>> --
>> 1.7.9.5
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/