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@web.de Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- 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; }
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@web.de Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Tested-by: Andrew Lunn andrew@lunn.ch
I tested this on a Kirkwood QNAP after removing the call to init_dma_coherent_pool_size().
Andrew
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);@@ -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
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@web.de
Please use Reported-by: Soeren Moch smoch@web.de
Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Tested-by: Andrew Lunn andrew@lunn.ch
I tested this on a Kirkwood QNAP after removing the call to init_dma_coherent_pool_size().
Andrew
Tested-by: Soeren Moch smoch@web.de
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;@@ -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
On Mon, Nov 12, 2012 at 10:48:02AM +0100, Soeren Moch wrote:
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@web.de
Please use Reported-by: Soeren Moch smoch@web.de
Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Tested-by: Andrew Lunn andrew@lunn.ch
I tested this on a Kirkwood QNAP after removing the call to init_dma_coherent_pool_size().
AndrewTested-by: Soeren Moch smoch@web.de
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!
Hi Soeren
Could you tell us what DVB devices you are using.
Thanks Andrew
On 12.11.2012 11:38, Andrew Lunn wrote:
On Mon, Nov 12, 2012 at 10:48:02AM +0100, Soeren Moch wrote:
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@web.de
Please use Reported-by: Soeren Moch smoch@web.de
Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Tested-by: Andrew Lunn andrew@lunn.ch
I tested this on a Kirkwood QNAP after removing the call to init_dma_coherent_pool_size().
AndrewTested-by: Soeren Moch smoch@web.de
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!
Hi Soeren
Could you tell us what DVB devices you are using.
Thanks Andrew
from lsusb: Bus 001 Device 005: ID 0ccd:00b2 TerraTec Electronic GmbH Bus 001 Device 006: ID 2040:5200 Hauppauge Bus 001 Device 009: ID 2304:0242 Pinnacle Systems, Inc.
If you want to check the drivers, I recommend to start with "em28xx".
Regards, Soeren
The patch:
mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls
makes these calls on Kirkword and Orion5x redundent. The drivers are not making atomic requests for coherent memory and hence the default pool size is now sufficient.
Signed-off-by: Andrew Lunn andrew@lunn.ch --- arch/arm/mach-kirkwood/common.c | 7 ------- arch/arm/mach-orion5x/common.c | 7 ------- 2 files changed, 14 deletions(-)
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c index 2c6c218..8080ceb 100644 --- a/arch/arm/mach-kirkwood/common.c +++ b/arch/arm/mach-kirkwood/common.c @@ -505,13 +505,6 @@ void __init kirkwood_wdt_init(void) void __init kirkwood_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE); - - /* - * Some Kirkwood devices allocate their coherent buffers from atomic - * context. Increase size of atomic coherent pool to make sure such - * the allocations won't fail. - */ - init_dma_coherent_pool_size(SZ_1M); }
int kirkwood_tclk; diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c index b3eb3da..3e07f52 100644 --- a/arch/arm/mach-orion5x/common.c +++ b/arch/arm/mach-orion5x/common.c @@ -194,13 +194,6 @@ void __init orion5x_wdt_init(void) void __init orion5x_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE); - - /* - * Some Orion5x devices allocate their coherent buffers from atomic - * context. Increase size of atomic coherent pool to make sure such - * the allocations won't fail. - */ - init_dma_coherent_pool_size(SZ_1M); }
int orion5x_tclk;
On Sun, Nov 11, 2012 at 06:31:19PM +0100, Andrew Lunn wrote:
The patch:
mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls
makes these calls on Kirkword and Orion5x redundent. The drivers are not making atomic requests for coherent memory and hence the default pool size is now sufficient.
Signed-off-by: Andrew Lunn andrew@lunn.ch
arch/arm/mach-kirkwood/common.c | 7 ------- arch/arm/mach-orion5x/common.c | 7 ------- 2 files changed, 14 deletions(-)
Andrew,
I'm going to hold onto this until we hear from the mm maintainers on the parent patch. If it doesn't make v3.8, I know we'll have to extend this patch to remove the similar code from mach-mvebu...
thx,
Jason.
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c index 2c6c218..8080ceb 100644 --- a/arch/arm/mach-kirkwood/common.c +++ b/arch/arm/mach-kirkwood/common.c @@ -505,13 +505,6 @@ void __init kirkwood_wdt_init(void) void __init kirkwood_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE);
- /*
* Some Kirkwood devices allocate their coherent buffers from atomic* context. Increase size of atomic coherent pool to make sure such* the allocations won't fail.*/- init_dma_coherent_pool_size(SZ_1M);
} int kirkwood_tclk; diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c index b3eb3da..3e07f52 100644 --- a/arch/arm/mach-orion5x/common.c +++ b/arch/arm/mach-orion5x/common.c @@ -194,13 +194,6 @@ void __init orion5x_wdt_init(void) void __init orion5x_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE);
- /*
* Some Orion5x devices allocate their coherent buffers from atomic* context. Increase size of atomic coherent pool to make sure such* the allocations won't fail.*/- init_dma_coherent_pool_size(SZ_1M);
} int orion5x_tclk; -- 1.7.10.4
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Andrew,
Now that we finally have Soren Moch's (sp? sorry) cache exhaustion solved, can we respin this for v3.11? I'd like to include mvebu as well if possible.
thx,
Jason.
On Sun, Nov 11, 2012 at 06:31:19PM +0100, Andrew Lunn wrote:
The patch:
mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls
makes these calls on Kirkword and Orion5x redundent. The drivers are not making atomic requests for coherent memory and hence the default pool size is now sufficient.
Signed-off-by: Andrew Lunn andrew@lunn.ch
arch/arm/mach-kirkwood/common.c | 7 ------- arch/arm/mach-orion5x/common.c | 7 ------- 2 files changed, 14 deletions(-)
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c index 2c6c218..8080ceb 100644 --- a/arch/arm/mach-kirkwood/common.c +++ b/arch/arm/mach-kirkwood/common.c @@ -505,13 +505,6 @@ void __init kirkwood_wdt_init(void) void __init kirkwood_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE);
- /*
* Some Kirkwood devices allocate their coherent buffers from atomic* context. Increase size of atomic coherent pool to make sure such* the allocations won't fail.*/- init_dma_coherent_pool_size(SZ_1M);
} int kirkwood_tclk; diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c index b3eb3da..3e07f52 100644 --- a/arch/arm/mach-orion5x/common.c +++ b/arch/arm/mach-orion5x/common.c @@ -194,13 +194,6 @@ void __init orion5x_wdt_init(void) void __init orion5x_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE);
- /*
* Some Orion5x devices allocate their coherent buffers from atomic* context. Increase size of atomic coherent pool to make sure such* the allocations won't fail.*/- init_dma_coherent_pool_size(SZ_1M);
} int orion5x_tclk; -- 1.7.10.4
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 04/17/2013 10:38 PM, Jason Cooper wrote:
Andrew,
Now that we finally have Soren Moch's (sp? sorry) cache exhaustion solved, can we respin this for v3.11? I'd like to include mvebu as well if possible.
For mvebu, I can do a follow-up patch, or Andrew can directly add it to his patch.
Regards,
thx,
Jason.
On Sun, Nov 11, 2012 at 06:31:19PM +0100, Andrew Lunn wrote:
The patch:
mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls
makes these calls on Kirkword and Orion5x redundent. The drivers are not making atomic requests for coherent memory and hence the default pool size is now sufficient.
Signed-off-by: Andrew Lunn andrew@lunn.ch
arch/arm/mach-kirkwood/common.c | 7 ------- arch/arm/mach-orion5x/common.c | 7 ------- 2 files changed, 14 deletions(-)
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c index 2c6c218..8080ceb 100644 --- a/arch/arm/mach-kirkwood/common.c +++ b/arch/arm/mach-kirkwood/common.c @@ -505,13 +505,6 @@ void __init kirkwood_wdt_init(void) void __init kirkwood_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE);
- /*
* Some Kirkwood devices allocate their coherent buffers from atomic* context. Increase size of atomic coherent pool to make sure such* the allocations won't fail.*/- init_dma_coherent_pool_size(SZ_1M);
} int kirkwood_tclk; diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c index b3eb3da..3e07f52 100644 --- a/arch/arm/mach-orion5x/common.c +++ b/arch/arm/mach-orion5x/common.c @@ -194,13 +194,6 @@ void __init orion5x_wdt_init(void) void __init orion5x_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE);
- /*
* Some Orion5x devices allocate their coherent buffers from atomic* context. Increase size of atomic coherent pool to make sure such* the allocations won't fail.*/- init_dma_coherent_pool_size(SZ_1M);
} int orion5x_tclk; -- 1.7.10.4
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Apr 17, 2013 at 10:49:53PM +0200, Gregory CLEMENT wrote:
On 04/17/2013 10:38 PM, Jason Cooper wrote:
Andrew,
Now that we finally have Soren Moch's (sp? sorry) cache exhaustion solved, can we respin this for v3.11? I'd like to include mvebu as well if possible.
For mvebu, I can do a follow-up patch, or Andrew can directly add it to his patch.
Applied to mvebu/cleanup with a few minor typo corrections and I added the hunk for mach-mvebu/.
thx,
Jason.
On Sun, Nov 11, 2012 at 06:31:19PM +0100, Andrew Lunn wrote:
The patch:
mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls
makes these calls on Kirkword and Orion5x redundent. The drivers are not making atomic requests for coherent memory and hence the default pool size is now sufficient.
Signed-off-by: Andrew Lunn andrew@lunn.ch
arch/arm/mach-kirkwood/common.c | 7 ------- arch/arm/mach-orion5x/common.c | 7 ------- 2 files changed, 14 deletions(-)
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c index 2c6c218..8080ceb 100644 --- a/arch/arm/mach-kirkwood/common.c +++ b/arch/arm/mach-kirkwood/common.c @@ -505,13 +505,6 @@ void __init kirkwood_wdt_init(void) void __init kirkwood_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE);
- /*
* Some Kirkwood devices allocate their coherent buffers from atomic* context. Increase size of atomic coherent pool to make sure such* the allocations won't fail.*/- init_dma_coherent_pool_size(SZ_1M);
} int kirkwood_tclk; diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c index b3eb3da..3e07f52 100644 --- a/arch/arm/mach-orion5x/common.c +++ b/arch/arm/mach-orion5x/common.c @@ -194,13 +194,6 @@ void __init orion5x_wdt_init(void) void __init orion5x_init_early(void) { orion_time_set_base(TIMER_VIRT_BASE);
- /*
* Some Orion5x devices allocate their coherent buffers from atomic* context. Increase size of atomic coherent pool to make sure such* the allocations won't fail.*/- init_dma_coherent_pool_size(SZ_1M);
} int orion5x_tclk; -- 1.7.10.4
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Marek,
I've added the maintainers for mm/*. Hopefully they can let us know if this is good for v3.8...
thx,
Jason.
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@web.de Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
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);@@ -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
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Sun, 18 Nov 2012 19:18:46 -0500 Jason Cooper jason@lakedaemon.net wrote:
I've added the maintainers for mm/*. Hopefully they can let us know if this is good for v3.8...
As Marek has inexplicably put this patch into linux-next via his tree, we don't appear to be getting a say in the matter!
The patch looks good to me. That open-coded wait loop predates the creation of bitkeeper tree(!) but doesn't appear to be needed. There will perhaps be some behavioural changes observable for GFP_KERNEL callers as dma_pool_alloc() will no longer dip into page reserves but I see nothing special about dma_pool_alloc() which justifies doing that anyway.
The patch makes pool->waitq and its manipulation obsolete, but it failed to remove all that stuff.
The changelog failed to describe the problem which Soren reported. That should be included, and as the problem sounds fairly serious we might decide to backport the fix into -stable kernels.
dma_pool_alloc()'s use of a local "struct dma_page *page" is distressing - MM developers very much expect a local called "page" to have type "struct page *". But that's a separate issue.
As this patch is already in -next and is stuck there for two more weeks I can't (or at least won't) merge this patch, so I can't help with any of the above.
Hello,
On 11/19/2012 11:48 PM, Andrew Morton wrote:
On Sun, 18 Nov 2012 19:18:46 -0500 Jason Cooper jason@lakedaemon.net wrote:
I've added the maintainers for mm/*. Hopefully they can let us know if this is good for v3.8...
As Marek has inexplicably put this patch into linux-next via his tree, we don't appear to be getting a say in the matter!
I've just put this patch to linux-next via my dma-mapping tree to give it some testing asap to check if other changes to arm dma-mapping are required or not.
The patch looks good to me. That open-coded wait loop predates the creation of bitkeeper tree(!) but doesn't appear to be needed. There will perhaps be some behavioural changes observable for GFP_KERNEL callers as dma_pool_alloc() will no longer dip into page reserves but I see nothing special about dma_pool_alloc() which justifies doing that anyway.
The patch makes pool->waitq and its manipulation obsolete, but it failed to remove all that stuff.
Right, I missed that part, I will update it asap.
The changelog failed to describe the problem which Soren reported. That should be included, and as the problem sounds fairly serious we might decide to backport the fix into -stable kernels.
Ok, I will extend the changelog.
dma_pool_alloc()'s use of a local "struct dma_page *page" is distressing - MM developers very much expect a local called "page" to have type "struct page *". But that's a separate issue.
I will prepare a separate patch cleaning it. I was also a bit surprised by such naming scheme, but it is probably related to the fact that this come has not been touched much since a very ancient times.
As this patch is already in -next and is stuck there for two more weeks I can't (or at least won't) merge this patch, so I can't help with any of the above.
I will fix both issues in the next version of the patch. Would like to merge it to your tree or should I keep it in my dma-mapping tree?
Best regards
On Tue, 20 Nov 2012 11:48:44 +0100 Marek Szyprowski m.szyprowski@samsung.com wrote:
As this patch is already in -next and is stuck there for two more weeks I can't (or at least won't) merge this patch, so I can't help with any of the above.
I will fix both issues in the next version of the patch. Would like to merge it to your tree or should I keep it in my dma-mapping tree?
The patch looks OK to me now (still wondering about the -stable question though).
It would be a bit of a pain for me to carry a patch which conflicts with one in linux-next, and this patch doesn't appear to conflict with the other pending dmapool.c patches in -mm so you may as well keep it in the dma-mapping tree now.
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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
This patch changes the dmapool code to correctly use gfp flags provided by the dmapool caller.
Reported-by: Soeren Moch smoch@web.de Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Tested-by: Andrew Lunn andrew@lunn.ch Tested-by: Soeren Moch smoch@web.de ---
changelog v2: - removed all waitq related stuff - extended commit message
mm/dmapool.c | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-)
diff --git a/mm/dmapool.c b/mm/dmapool.c index c5ab33b..da1b0f0 100644 --- a/mm/dmapool.c +++ b/mm/dmapool.c @@ -50,7 +50,6 @@ struct dma_pool { /* the pool */ size_t allocation; size_t boundary; char name[32]; - wait_queue_head_t waitq; struct list_head pools; };
@@ -62,8 +61,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 @@ -172,7 +169,6 @@ struct dma_pool *dma_pool_create(const char *name, struct device *dev, retval->size = size; retval->boundary = boundary; retval->allocation = allocation; - init_waitqueue_head(&retval->waitq);
if (dev) { int ret; @@ -227,7 +223,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 +310,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 +334,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; } @@ -435,8 +420,6 @@ void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t dma) page->in_use--; *(int *)vaddr = page->offset; page->offset = offset; - if (waitqueue_active(&pool->waitq)) - wake_up_locked(&pool->waitq); /* * Resist a temptation to do * if (!is_page_busy(page)) pool_free_page(pool, page);
On Tue, 20 Nov 2012 15:31:45 +0100 Marek Szyprowski m.szyprowski@samsung.com 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
Is this problem serious enough to justify merging the patch into 3.7? And into -stable kernels?
On Tue, Nov 20, 2012 at 11:33:25AM -0800, Andrew Morton wrote:
On Tue, 20 Nov 2012 15:31:45 +0100 Marek Szyprowski m.szyprowski@samsung.com 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
Is this problem serious enough to justify merging the patch into 3.7? And into -stable kernels?
kirkwood and orion5x currently have the following code in their early init:
/* * Some Kirkwood devices allocate their coherent buffers from atomic * context. Increase size of atomic coherent pool to make sure such the * allocations won't fail. */ init_dma_coherent_pool_size(SZ_1M);
We have a pending patch to do the same for mvebu (new armv7 Marvell SoCs). There is at least one reported real world case where even the above isn't sufficient [1].
thx,
Jason.
Hello,
On 11/20/2012 8:33 PM, Andrew Morton wrote:
On Tue, 20 Nov 2012 15:31:45 +0100 Marek Szyprowski m.szyprowski@samsung.com 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
Is this problem serious enough to justify merging the patch into 3.7? And into -stable kernels?
I wonder if it is a good idea to merge such change at the end of current -rc period. It changes the behavior of dma pool allocations and I bet there might be some drivers which don't care much about passed gfp flags, as for ages it simply worked for them, even if the allocations were done from atomic context. What do You think? Technically it is also not a pure bugfix, so imho it shouldn't be considered for -stable.
On the other hand at least for ARM users of sata_mv driver (which is just an innocent client of dma pool, correctly passing GFP_KERNEL flag) it would solve the issues related to shortage of atomic pool for dma allocations what might justify pushing it to 3.7.
Best regards
On Wed, 21 Nov 2012 09:08:52 +0100 Marek Szyprowski m.szyprowski@samsung.com wrote:
Hello,
On 11/20/2012 8:33 PM, Andrew Morton wrote:
On Tue, 20 Nov 2012 15:31:45 +0100 Marek Szyprowski m.szyprowski@samsung.com 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
Is this problem serious enough to justify merging the patch into 3.7? And into -stable kernels?
I wonder if it is a good idea to merge such change at the end of current -rc period.
I'm not sure what you mean by this.
But what we do sometimes if we think a patch needs a bit more real-world testing before backporting is to merge it into -rc1 in the normal merge window, and tag it for -stable backporting. That way it gets a few weeks(?) testing in mainline before getting backported.
Hello,
On 11/21/2012 9:36 AM, Andrew Morton wrote:
On Wed, 21 Nov 2012 09:08:52 +0100 Marek Szyprowski m.szyprowski@samsung.com wrote:
Hello,
On 11/20/2012 8:33 PM, Andrew Morton wrote:
On Tue, 20 Nov 2012 15:31:45 +0100 Marek Szyprowski m.szyprowski@samsung.com 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
Is this problem serious enough to justify merging the patch into 3.7? And into -stable kernels?
I wonder if it is a good idea to merge such change at the end of current -rc period.
I'm not sure what you mean by this.
But what we do sometimes if we think a patch needs a bit more real-world testing before backporting is to merge it into -rc1 in the normal merge window, and tag it for -stable backporting. That way it gets a few weeks(?) testing in mainline before getting backported.
I just wondered that if it gets merged to v3.7-rc7 there won't be much time for real-world testing before final v3.7 release. This patch is in linux-next for over a week and I'm not aware of any issues, but -rc releases gets much more attention and testing than linux-next tree.
If You think it's fine to put such change to v3.7-rc7 I will send a pull request and tag it for stable asap.
Best regards
On Wed, 21 Nov 2012 10:20:07 +0100 Marek Szyprowski m.szyprowski@samsung.com wrote:
Hello,
On 11/21/2012 9:36 AM, Andrew Morton wrote:
On Wed, 21 Nov 2012 09:08:52 +0100 Marek Szyprowski m.szyprowski@samsung.com wrote:
Hello,
On 11/20/2012 8:33 PM, Andrew Morton wrote:
On Tue, 20 Nov 2012 15:31:45 +0100 Marek Szyprowski m.szyprowski@samsung.com 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
Is this problem serious enough to justify merging the patch into 3.7? And into -stable kernels?
I wonder if it is a good idea to merge such change at the end of current -rc period.
I'm not sure what you mean by this.
But what we do sometimes if we think a patch needs a bit more real-world testing before backporting is to merge it into -rc1 in the normal merge window, and tag it for -stable backporting. That way it gets a few weeks(?) testing in mainline before getting backported.
I just wondered that if it gets merged to v3.7-rc7 there won't be much time for real-world testing before final v3.7 release. This patch is in linux-next for over a week and I'm not aware of any issues, but -rc releases gets much more attention and testing than linux-next tree.
If You think it's fine to put such change to v3.7-rc7 I will send a pull request and tag it for stable asap.
What I'm suggesting is that it be merged for 3.8-rc1 with a -stable tag, then it will be backported into 3.7.x later on.
On 11/21/2012 8:17 PM, Andrew Morton wrote:
On Wed, 21 Nov 2012 10:20:07 +0100 Marek Szyprowski m.szyprowski@samsung.com wrote:
On 11/21/2012 9:36 AM, Andrew Morton wrote:
On Wed, 21 Nov 2012 09:08:52 +0100 Marek Szyprowski m.szyprowski@samsung.com wrote:
On 11/20/2012 8:33 PM, Andrew Morton wrote:
On Tue, 20 Nov 2012 15:31:45 +0100 Marek Szyprowski m.szyprowski@samsung.com 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
Is this problem serious enough to justify merging the patch into 3.7? And into -stable kernels?
I wonder if it is a good idea to merge such change at the end of current -rc period.
I'm not sure what you mean by this.
But what we do sometimes if we think a patch needs a bit more real-world testing before backporting is to merge it into -rc1 in the normal merge window, and tag it for -stable backporting. That way it gets a few weeks(?) testing in mainline before getting backported.
I just wondered that if it gets merged to v3.7-rc7 there won't be much time for real-world testing before final v3.7 release. This patch is in linux-next for over a week and I'm not aware of any issues, but -rc releases gets much more attention and testing than linux-next tree.
If You think it's fine to put such change to v3.7-rc7 I will send a pull request and tag it for stable asap.
What I'm suggesting is that it be merged for 3.8-rc1 with a -stable tag, then it will be backported into 3.7.x later on.
OK, I will push it to v3.8-rc1 then and tag for stable.
Best regards
On 20.11.2012 15:31, 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
This patch changes the dmapool code to correctly use gfp flags provided by the dmapool caller.
Reported-by: Soeren Moch smoch@web.de Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Tested-by: Andrew Lunn andrew@lunn.ch Tested-by: Soeren Moch smoch@web.de
Now I tested linux-3.7.1 (this patch is included there) on my Marvell Kirkwood system. I still see
ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!
after several hours of runtime under heavy load with SATA and DVB-Sticks (em28xx / drxk and dib0700).
As already reported earlier this patch improved the behavior compared to linux-3.6.x and 3.7.0 (error after several ten minutes runtime), but I still see a regression compared to linux-3.5.x. With this kernel the same system with same workload runs flawlessly.
Regards, Soeren
Greg,
I've added you to the this thread hoping for a little insight into USB drivers and their use of coherent and GFP_ATOMIC. Am I barking up the wrong tree by looking a the drivers?
On Mon, Jan 14, 2013 at 12:56:57PM +0100, Soeren Moch wrote:
On 20.11.2012 15:31, 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
This patch changes the dmapool code to correctly use gfp flags provided by the dmapool caller.
Reported-by: Soeren Moch smoch@web.de Reported-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Tested-by: Andrew Lunn andrew@lunn.ch Tested-by: Soeren Moch smoch@web.de
Now I tested linux-3.7.1 (this patch is included there) on my Marvell Kirkwood system. I still see
ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!
after several hours of runtime under heavy load with SATA and DVB-Sticks (em28xx / drxk and dib0700).
Could you try running the system w/o the em28xx stick and see how it goes with v3.7.1?
thx,
Jason.
As already reported earlier this patch improved the behavior compared to linux-3.6.x and 3.7.0 (error after several ten minutes runtime), but I still see a regression compared to linux-3.5.x. With this kernel the same system with same workload runs flawlessly.
Regards, Soeren
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Jan 15, 2013 at 11:56:42AM -0500, Jason Cooper wrote:
Greg,
I've added you to the this thread hoping for a little insight into USB drivers and their use of coherent and GFP_ATOMIC. Am I barking up the wrong tree by looking a the drivers?
I don't understand, which drivers are you referring to? USB host controller drivers, or the "normal" drivers? Most USB drivers use GFP_ATOMIC if they are creating memory during their URB callback path, as that is interrupt context. But it shouldn't be all that bad, and the USB core hasn't changed in a while, so something else must be causing this.
greg k-h
On Tue, Jan 15, 2013 at 09:50:20AM -0800, Greg KH wrote:
On Tue, Jan 15, 2013 at 11:56:42AM -0500, Jason Cooper wrote:
Greg,
I've added you to the this thread hoping for a little insight into USB drivers and their use of coherent and GFP_ATOMIC. Am I barking up the wrong tree by looking a the drivers?
I don't understand, which drivers are you referring to? USB host controller drivers, or the "normal" drivers?
Sorry I wasn't clear, I was referring specifically to the usb dvb drivers em28xx, drxk and dib0700. These are the drivers reported to be in heavy use when the error occurs.
sata_mv is also in use, however no other users of sata_mv have reported problems. Including myself. ;-)
Most USB drivers use GFP_ATOMIC if they are creating memory during their URB callback path, as that is interrupt context. But it shouldn't be all that bad, and the USB core hasn't changed in a while, so something else must be causing this.
Agreed, so I went and did more reading. The key piece of the puzzle that I was missing was in arch/arm/mm/dma-mapping.c 660-684.
/* * Allocate DMA-coherent memory space and return both the kernel * remapped * virtual and bus address for that space. */ void *arm_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs) { pgprot_t prot = __get_dma_pgprot(attrs, pgprot_kernel); void *memory;
if (dma_alloc_from_coherent(dev, size, handle, &memory)) return memory;
return __dma_alloc(dev, size, handle, gfp, prot, false, __builtin_return_address(0)); }
static void *arm_coherent_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs) { pgprot_t prot = __get_dma_pgprot(attrs, pgprot_kernel); void *memory;
if (dma_alloc_from_coherent(dev, size, handle, &memory)) return memory;
return __dma_alloc(dev, size, handle, gfp, prot, true, __builtin_return_address(0)); }
My understanding of this code is that when a driver requests dma memory, we will first try to alloc from the per-driver pool. If that fails, we will then attempt to allocate from the atomic_pool.
Once the atomic_pool is exhausted, we get the error:
ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!
If my understanding is correct, one of the drivers (most likely one) either asks for too small of a dma buffer, or is not properly deallocating blocks from the per-device pool. Either case leads to exhaustion, and falling back to the atomic pool. Which subsequently gets wiped out as well.
Am I on the right track?
thx,
Jason.
Soeren,
On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
If my understanding is correct, one of the drivers (most likely one) either asks for too small of a dma buffer, or is not properly deallocating blocks from the per-device pool. Either case leads to exhaustion, and falling back to the atomic pool. Which subsequently gets wiped out as well.
If my hunch is right, could you please try each of the three dvb drivers in turn and see which one (or more than one) causes the error?
thx,
Jason.
On 15.01.2013 22:56, Jason Cooper wrote:
Soeren,
On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
If my understanding is correct, one of the drivers (most likely one) either asks for too small of a dma buffer, or is not properly deallocating blocks from the per-device pool. Either case leads to exhaustion, and falling back to the atomic pool. Which subsequently gets wiped out as well.
If my hunch is right, could you please try each of the three dvb drivers in turn and see which one (or more than one) causes the error?
In fact I use only 2 types of DVB sticks: em28xx usb bridge plus drxk demodulator, and dib0700 usb bridge plus dib7000p demod.
I would bet for em28xx causing the error, but this is not thoroughly tested. Unfortunately testing with removed sticks is not easy, because this is a production system and disabling some services for the long time we need to trigger this error will certainly result in unhappy users.
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
In linux-3.5.x there is no such problem. Can we use all available memory for dma buffers here on armv5 architectures, in contrast to newer kernels?
Regards, Soeren
Soeren,
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote:
On 15.01.2013 22:56, Jason Cooper wrote:
On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
If my understanding is correct, one of the drivers (most likely one) either asks for too small of a dma buffer, or is not properly deallocating blocks from the per-device pool. Either case leads to exhaustion, and falling back to the atomic pool. Which subsequently gets wiped out as well.
If my hunch is right, could you please try each of the three dvb drivers in turn and see which one (or more than one) causes the error?
In fact I use only 2 types of DVB sticks: em28xx usb bridge plus drxk demodulator, and dib0700 usb bridge plus dib7000p demod.
I would bet for em28xx causing the error, but this is not thoroughly tested. Unfortunately testing with removed sticks is not easy, because this is a production system and disabling some services for the long time we need to trigger this error will certainly result in unhappy users.
Just out of curiosity, what board is it?
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
In linux-3.5.x there is no such problem. Can we use all available memory for dma buffers here on armv5 architectures, in contrast to newer kernels?
Were the loads exactly the same when you tested 3.5.x? I looked at the changes from v3.5 to v3.7.1 for all four drivers you mentioned as well as sata_mv.
The biggest thing I see is that all of the media drivers got shuffled around into their own subdirectories after v3.5. 'git show -M 0c0d06c' shows it was a clean copy of all the files.
What would be most helpful is if you could do a git bisect between v3.5.x (working) and the oldest version where you know it started failing (v3.7.1 or earlier if you know it).
thx,
Jason.
On 16.01.2013 03:40, Jason Cooper wrote:
Soeren,
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote:
On 15.01.2013 22:56, Jason Cooper wrote:
On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
If my understanding is correct, one of the drivers (most likely one) either asks for too small of a dma buffer, or is not properly deallocating blocks from the per-device pool. Either case leads to exhaustion, and falling back to the atomic pool. Which subsequently gets wiped out as well.
If my hunch is right, could you please try each of the three dvb drivers in turn and see which one (or more than one) causes the error?
In fact I use only 2 types of DVB sticks: em28xx usb bridge plus drxk demodulator, and dib0700 usb bridge plus dib7000p demod.
I would bet for em28xx causing the error, but this is not thoroughly tested. Unfortunately testing with removed sticks is not easy, because this is a production system and disabling some services for the long time we need to trigger this error will certainly result in unhappy users.
Just out of curiosity, what board is it?
The kirkwood board? A modified Guruplug Server Plus.
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
OK, maybe I can try this.
In linux-3.5.x there is no such problem. Can we use all available memory for dma buffers here on armv5 architectures, in contrast to newer kernels?
Were the loads exactly the same when you tested 3.5.x?
Exactly the same, yes.
I looked at the changes from v3.5 to v3.7.1 for all four drivers you mentioned as well as sata_mv.
The biggest thing I see is that all of the media drivers got shuffled around into their own subdirectories after v3.5. 'git show -M 0c0d06c' shows it was a clean copy of all the files.
What would be most helpful is if you could do a git bisect between v3.5.x (working) and the oldest version where you know it started failing (v3.7.1 or earlier if you know it).
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore). I'm not very familiar with arm mm code, and from the patch itself I cannot understand what's different. Maybe CONFIG_CMA is default also for armv5 (not only v6) now? But I might be totally wrong here, maybe someone of the mm experts can explain the difference?
Regards, Soeren
On 16.01.2013 04:24, Soeren Moch wrote:
On 16.01.2013 03:40, Jason Cooper wrote:
Soeren,
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote:
On 15.01.2013 22:56, Jason Cooper wrote:
On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
If my understanding is correct, one of the drivers (most likely one) either asks for too small of a dma buffer, or is not properly deallocating blocks from the per-device pool. Either case leads to exhaustion, and falling back to the atomic pool. Which subsequently gets wiped out as well.
If my hunch is right, could you please try each of the three dvb drivers in turn and see which one (or more than one) causes the error?
In fact I use only 2 types of DVB sticks: em28xx usb bridge plus drxk demodulator, and dib0700 usb bridge plus dib7000p demod.
I would bet for em28xx causing the error, but this is not thoroughly tested. Unfortunately testing with removed sticks is not easy, because this is a production system and disabling some services for the long time we need to trigger this error will certainly result in unhappy users.
OK, I could trigger the error ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter! only with em28xx sticks and sata, dib0700 sticks removed.
Just out of curiosity, what board is it?
The kirkwood board? A modified Guruplug Server Plus.
em28xx sticks: "TerraTec Cinergy HTC Stick HD" and "PCTV Quatro Stick" dib0700 sticks: "WinTV-NOVA-TD Stick"
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
OK, maybe I can try this.
In linux-3.5.x there is no such problem. Can we use all available memory for dma buffers here on armv5 architectures, in contrast to newer kernels?
Were the loads exactly the same when you tested 3.5.x?
Exactly the same, yes.
I looked at the changes from v3.5 to v3.7.1 for all four drivers you mentioned as well as sata_mv.
The biggest thing I see is that all of the media drivers got shuffled around into their own subdirectories after v3.5. 'git show -M 0c0d06c' shows it was a clean copy of all the files.
What would be most helpful is if you could do a git bisect between v3.5.x (working) and the oldest version where you know it started failing (v3.7.1 or earlier if you know it).
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore).
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
I'm not very familiar with arm mm code, and from the patch itself I cannot understand what's different. Maybe CONFIG_CMA is default also for armv5 (not only v6) now? But I might be totally wrong here, maybe someone of the mm experts can explain the difference?
Regards, Soeren
On Wed, Jan 16, 2013 at 09:55:55AM +0100, Soeren Moch wrote:
On 16.01.2013 04:24, Soeren Moch wrote:
On 16.01.2013 03:40, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote:
On 15.01.2013 22:56, Jason Cooper wrote:
On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
OK, I could trigger the error ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter! only with em28xx sticks and sata, dib0700 sticks removed.
Did you test the reverse scenario? ie dib0700 with sata_mv and no em28xx.
What kind of throughput are you pushing to the sata disk?
What would be most helpful is if you could do a git bisect between v3.5.x (working) and the oldest version where you know it started failing (v3.7.1 or earlier if you know it).
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore).
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
Of the four drivers you listed, none are using dma. sata_mv is the only one.
If one is to believe the comments in sata_mv.c:~151, then the alignment is wrong for the sg_tbl_pool.
Could you please try the following patch?
thx,
Jason.
---8<----------
From 566c7e30285e4c31d76724ea4811b016b753f24f Mon Sep 17 00:00:00 2001
From: Jason Cooper jason@lakedaemon.net Date: Wed, 16 Jan 2013 15:43:37 +0000 Subject: [PATCH] ata: sata_mv: fix sg_tbl_pool alignment
If the comment is to be believed, the alignment should be 16B, and the size 4K. The current code sets both to 4K. On some arm boards (kirkwood), this causes:
ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!
Set alignment to 16B to prevent exhausting the atomic_pool.
Signed-off-by: Jason Cooper jason@lakedaemon.net --- drivers/ata/sata_mv.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index 68f4fb5..e2e5a8a 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -148,6 +148,9 @@ enum { * CRPB needs alignment on a 256B boundary. Size == 256B * ePRD (SG) entries need alignment on a 16B boundary. Size == 16B */ + MV_CRQB_Q_ALIGN = 1024, + MV_CRPB_Q_ALIGN = 256, + MV_SG_TBL_ALIGN = 16, MV_CRQB_Q_SZ = (32 * MV_MAX_Q_DEPTH), MV_CRPB_Q_SZ = (8 * MV_MAX_Q_DEPTH), MV_MAX_SG_CT = 256, @@ -3975,17 +3978,17 @@ done: static int mv_create_dma_pools(struct mv_host_priv *hpriv, struct device *dev) { hpriv->crqb_pool = dmam_pool_create("crqb_q", dev, MV_CRQB_Q_SZ, - MV_CRQB_Q_SZ, 0); + MV_CRQB_Q_ALIGN, 0); if (!hpriv->crqb_pool) return -ENOMEM;
hpriv->crpb_pool = dmam_pool_create("crpb_q", dev, MV_CRPB_Q_SZ, - MV_CRPB_Q_SZ, 0); + MV_CRPB_Q_ALIGN, 0); if (!hpriv->crpb_pool) return -ENOMEM;
hpriv->sg_tbl_pool = dmam_pool_create("sg_tbl", dev, MV_SG_TBL_SZ, - MV_SG_TBL_SZ, 0); + MV_SG_TBL_ALIGN, 0); if (!hpriv->sg_tbl_pool) return -ENOMEM;
On 16.01.2013 16:50, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 09:55:55AM +0100, Soeren Moch wrote:
On 16.01.2013 04:24, Soeren Moch wrote:
On 16.01.2013 03:40, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote:
On 15.01.2013 22:56, Jason Cooper wrote:
On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
OK, I could trigger the error ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter! only with em28xx sticks and sata, dib0700 sticks removed.
Did you test the reverse scenario? ie dib0700 with sata_mv and no em28xx.
Maybe I can test this next night.
What kind of throughput are you pushing to the sata disk?
Close to nothing. In the last test I had the root filesystem running on the sata disk plus a few 10 megabytes per hour.
What would be most helpful is if you could do a git bisect between v3.5.x (working) and the oldest version where you know it started failing (v3.7.1 or earlier if you know it).
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore).
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
Of the four drivers you listed, none are using dma. sata_mv is the only one.
usb_core is doing the actual DMA for the usb bridge drivers, I think.
If one is to believe the comments in sata_mv.c:~151, then the alignment is wrong for the sg_tbl_pool.
Could you please try the following patch?
OK, what should I test first, the setup from last night (em28xx, no dib0700) plus your patch, or the reverse setup (dib0700, no em28xx) without your patch, or my normal setting (all dvb sticks) plus your patch?
Regards, Soeren
On Wed, Jan 16, 2013 at 06:05:59PM +0100, Soeren Moch wrote:
On 16.01.2013 16:50, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 09:55:55AM +0100, Soeren Moch wrote:
On 16.01.2013 04:24, Soeren Moch wrote:
On 16.01.2013 03:40, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote:
On 15.01.2013 22:56, Jason Cooper wrote: >On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
OK, I could trigger the error ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter! only with em28xx sticks and sata, dib0700 sticks removed.
Did you test the reverse scenario? ie dib0700 with sata_mv and no em28xx.
Maybe I can test this next night.
Please do, this will tell us if it is in the USB drivers or lower (something in common).
What would be most helpful is if you could do a git bisect between v3.5.x (working) and the oldest version where you know it started failing (v3.7.1 or earlier if you know it).
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore).
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
Of the four drivers you listed, none are using dma. sata_mv is the only one.
usb_core is doing the actual DMA for the usb bridge drivers, I think.
Yes, my mistake. I'd like to attribute that statement to pre-coffee rambling. :-)
If one is to believe the comments in sata_mv.c:~151, then the alignment is wrong for the sg_tbl_pool.
Could you please try the following patch?
OK, what should I test first, the setup from last night (em28xx, no dib0700) plus your patch, or the reverse setup (dib0700, no em28xx) without your patch, or my normal setting (all dvb sticks) plus your patch?
if testing time is limited, please do the test I outlined at the top of this email. I've been digging more into the dma code and while I think the patch is correct, I don't see where it would fix your problem (yet).
thx,
Jason.
On Wed, Jan 16, 2013 at 09:55:55AM +0100, Soeren Moch wrote:
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
Could you send the output of:
lsusb -v -d VEND:PROD
for the em28xx?
thx,
Jason.
On 16.01.2013 19:35, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 09:55:55AM +0100, Soeren Moch wrote:
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
Could you send the output of:
lsusb -v -d VEND:PROD
for the em28xx?
thx,
Jason.
Here is the lsusb output for both of my em28xx sticks.
Regards, Soeren
Bus 001 Device 005: ID 0ccd:00b2 TerraTec Electronic GmbH Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 idVendor 0x0ccd TerraTec Electronic GmbH idProduct 0x00b2 bcdDevice 1.00 iManufacturer 3 TERRATEC iProduct 1 Cinergy HTC Stick iSerial 2 123456789ABCD bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 305 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 1 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 2 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0ad0 2x 720 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 3 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0c00 2x 1024 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 4 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1300 3x 768 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 5 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1380 3x 896 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 6 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x13c0 3x 960 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 7 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1400 3x 1024 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Device Qualifier (for other device speed): bLength 10 bDescriptorType 6 bcdUSB 2.00 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 bNumConfigurations 1 Device Status: 0x0000 (Bus Powered)
Bus 001 Device 009: ID 2304:0242 Pinnacle Systems, Inc. Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 idVendor 0x2304 Pinnacle Systems, Inc. idProduct 0x0242 bcdDevice 1.00 iManufacturer 1 Pinnacle Systems iProduct 2 PCTV 510e iSerial 3 123456789012 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 305 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 1 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 2 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0ad0 2x 720 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 3 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0c00 2x 1024 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 4 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1300 3x 768 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 5 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1380 3x 896 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 6 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x13c0 3x 960 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 7 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1400 3x 1024 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Device Qualifier (for other device speed): bLength 10 bDescriptorType 6 bcdUSB 2.00 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 bNumConfigurations 1 Device Status: 0x0000 (Bus Powered)
On 16.01.2013 18:52, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 06:05:59PM +0100, Soeren Moch wrote:
On 16.01.2013 16:50, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 09:55:55AM +0100, Soeren Moch wrote:
On 16.01.2013 04:24, Soeren Moch wrote:
On 16.01.2013 03:40, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote: > On 15.01.2013 22:56, Jason Cooper wrote: >> On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
OK, I could trigger the error ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter! only with em28xx sticks and sata, dib0700 sticks removed.
Did you test the reverse scenario? ie dib0700 with sata_mv and no em28xx.
Maybe I can test this next night.
Please do, this will tell us if it is in the USB drivers or lower (something in common).
What would be most helpful is if you could do a git bisect between v3.5.x (working) and the oldest version where you know it started failing (v3.7.1 or earlier if you know it).
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore).
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
Of the four drivers you listed, none are using dma. sata_mv is the only one.
usb_core is doing the actual DMA for the usb bridge drivers, I think.
Yes, my mistake. I'd like to attribute that statement to pre-coffee rambling. :-)
If one is to believe the comments in sata_mv.c:~151, then the alignment is wrong for the sg_tbl_pool.
Could you please try the following patch?
OK, what should I test first, the setup from last night (em28xx, no dib0700) plus your patch, or the reverse setup (dib0700, no em28xx) without your patch, or my normal setting (all dvb sticks) plus your patch?
if testing time is limited, please do the test I outlined at the top of this email. I've been digging more into the dma code and while I think the patch is correct, I don't see where it would fix your problem (yet).
Unfortunately test time is limited, and the test has to run about 10 hours to trigger the error.
I also think that sata is not causing the problem. Maybe your patch even goes in the wrong direction. Perhaps the dma memory pool is not too small, there might be enough memory available, but it is too much fragmented to satisfy larger block allocations. With different drivers allocating totally different block sizes aligned to bytes or words, perhaps we end up with lots of very small free blocks in the dma pool after several hours of runtime? So maybe it would help to align all allocations in the dma pool to 256B?
Regards, Soeren
On 16.01.2013 18:52, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 06:05:59PM +0100, Soeren Moch wrote:
On 16.01.2013 16:50, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 09:55:55AM +0100, Soeren Moch wrote:
On 16.01.2013 04:24, Soeren Moch wrote:
On 16.01.2013 03:40, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote: > On 15.01.2013 22:56, Jason Cooper wrote: >> On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
OK, I could trigger the error ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter! only with em28xx sticks and sata, dib0700 sticks removed.
Did you test the reverse scenario? ie dib0700 with sata_mv and no em28xx.
Maybe I can test this next night.
Please do, this will tell us if it is in the USB drivers or lower (something in common).
Until now there is no error with dib0700 + sata, without em28xx.
But to be sure that there is absolutely no problem with this setting we probably need additional testing hours. BTW, these dib0700 sticks use usb bulk transfers (and maybe smaller dma buffers?).
Regards, Soeren
On 16.01.2013 09:55, Soeren Moch wrote:
On 16.01.2013 04:24, Soeren Moch wrote:
On 16.01.2013 03:40, Jason Cooper wrote:
Soeren,
On Wed, Jan 16, 2013 at 01:17:59AM +0100, Soeren Moch wrote:
On 15.01.2013 22:56, Jason Cooper wrote:
On Tue, Jan 15, 2013 at 03:16:17PM -0500, Jason Cooper wrote:
If my understanding is correct, one of the drivers (most likely one) either asks for too small of a dma buffer, or is not properly deallocating blocks from the per-device pool. Either case leads to exhaustion, and falling back to the atomic pool. Which subsequently gets wiped out as well.
If my hunch is right, could you please try each of the three dvb drivers in turn and see which one (or more than one) causes the error?
In fact I use only 2 types of DVB sticks: em28xx usb bridge plus drxk demodulator, and dib0700 usb bridge plus dib7000p demod.
I would bet for em28xx causing the error, but this is not thoroughly tested. Unfortunately testing with removed sticks is not easy, because this is a production system and disabling some services for the long time we need to trigger this error will certainly result in unhappy users.
OK, I could trigger the error ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter! only with em28xx sticks and sata, dib0700 sticks removed.
Just out of curiosity, what board is it?
The kirkwood board? A modified Guruplug Server Plus.
em28xx sticks: "TerraTec Cinergy HTC Stick HD" and "PCTV Quatro Stick" dib0700 sticks: "WinTV-NOVA-TD Stick"
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
OK, maybe I can try this.
In linux-3.5.x there is no such problem. Can we use all available memory for dma buffers here on armv5 architectures, in contrast to newer kernels?
Were the loads exactly the same when you tested 3.5.x?
Exactly the same, yes.
I looked at the changes from v3.5 to v3.7.1 for all four drivers you mentioned as well as sata_mv.
The biggest thing I see is that all of the media drivers got shuffled around into their own subdirectories after v3.5. 'git show -M 0c0d06c' shows it was a clean copy of all the files.
What would be most helpful is if you could do a git bisect between v3.5.x (working) and the oldest version where you know it started failing (v3.7.1 or earlier if you know it).
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore).
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
The em28xx sticks are using isochronous usb transfers. Is there a special handling for that?
I'm not very familiar with arm mm code, and from the patch itself I cannot understand what's different. Maybe CONFIG_CMA is default also for armv5 (not only v6) now? But I might be totally wrong here, maybe someone of the mm experts can explain the difference?
Regards, Soeren
On Wed, Jan 16, 2013 at 06:32:09PM +0100, Soeren Moch wrote:
On 16.01.2013 09:55, Soeren Moch wrote:
On 16.01.2013 04:24, Soeren Moch wrote:
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore).
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
The em28xx sticks are using isochronous usb transfers. Is there a special handling for that?
I'm looking at that now. It looks like the em28xx wants (as a maximum) 655040 bytes (em28xx-core.c:1088). There are 5 transfer buffers, with 64 max packets and 2047 max packet size (runtime reported max & 0x7ff).
If it actually needs all of that, then the answer may be to just increase coherent_pool= when using that driver. I'll keep digging.
thx,
Jason.
On 16.01.2013 18:47, Jason Cooper wrote:
On Wed, Jan 16, 2013 at 06:32:09PM +0100, Soeren Moch wrote:
On 16.01.2013 09:55, Soeren Moch wrote:
On 16.01.2013 04:24, Soeren Moch wrote:
I did not bisect it, but Marek mentioned earlier that commit e9da6e9905e639b0f842a244bc770b48ad0523e9 in Linux v3.6-rc1 introduced new code for dma allocations. This is probably the root cause for the new (mis-)behavior (due to my tests 3.6.0 is not working anymore).
I don't want to say that Mareks patch is wrong, probably it triggers a bug somewhere else! (in em28xx?)
The em28xx sticks are using isochronous usb transfers. Is there a special handling for that?
I'm looking at that now. It looks like the em28xx wants (as a maximum) 655040 bytes (em28xx-core.c:1088). There are 5 transfer buffers, with 64 max packets and 2047 max packet size (runtime reported max & 0x7ff).
If it actually needs all of that, then the answer may be to just increase coherent_pool= when using that driver. I'll keep digging.
I already tested with 4M coherent pool size and could not see significant improvement. Would it make sense to further increase the buffer size?
Regards, Soeren
On Wednesday 16 January 2013, Soeren Moch wrote:
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
OK, maybe I can try this.
Any success with this? It should at least tell you if there is a memory leak in one of the drivers.
Arnd
On 17.01.2013 11:49, Arnd Bergmann wrote:
On Wednesday 16 January 2013, Soeren Moch wrote:
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
OK, maybe I can try this.
Any success with this? It should at least tell you if there is a memory leak in one of the drivers.
Not yet, sorry. I have to do all the tests in my limited spare time. Can you tell me what to search for in the debug output?
Soeren
On Thursday 17 January 2013, Soeren Moch wrote:
On 17.01.2013 11:49, Arnd Bergmann wrote:
On Wednesday 16 January 2013, Soeren Moch wrote:
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
OK, maybe I can try this.
Any success with this? It should at least tell you if there is a memory leak in one of the drivers.
Not yet, sorry. I have to do all the tests in my limited spare time. Can you tell me what to search for in the debug output?
Actually now that I've looked closer, you can't immediately see all the mappings as I thought.
But please try enabling DMA_API_DEBUG in combination with this one-line patch:
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6b2fb87..3df74ac 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -497,6 +497,7 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page) pr_err_once("ERROR: %u KiB atomic DMA coherent pool is too small!\n" "Please increase it with coherent_pool= kernel parameter!\n", (unsigned)pool->size / 1024); + debug_dma_dump_mappings(NULL); } spin_unlock_irqrestore(&pool->lock, flags);
That will show every single allocation that is currently active. This lets you see where all the memory went, and if there is a possible leak or excessive fragmentation.
Arnd
On 17.01.2013 21:26, Arnd Bergmann wrote:
On Thursday 17 January 2013, Soeren Moch wrote:
On 17.01.2013 11:49, Arnd Bergmann wrote:
On Wednesday 16 January 2013, Soeren Moch wrote:
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
OK, maybe I can try this.
Any success with this? It should at least tell you if there is a memory leak in one of the drivers.
Not yet, sorry. I have to do all the tests in my limited spare time. Can you tell me what to search for in the debug output?
Actually now that I've looked closer, you can't immediately see all the mappings as I thought.
But please try enabling DMA_API_DEBUG in combination with this one-line patch:
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6b2fb87..3df74ac 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -497,6 +497,7 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page) pr_err_once("ERROR: %u KiB atomic DMA coherent pool is too small!\n" "Please increase it with coherent_pool= kernel parameter!\n", (unsigned)pool->size / 1024);
debug_dma_dump_mappings(NULL);That will show every single allocation that is currently active. This lets you see where all the memory went, and if there is a possible leak or excessive fragmentation.
Arnd
Please find attached a debug log generated with your patch.
I used the sata disk and two em28xx dvb sticks, no other usb devices, no ethernet cable connected, tuners on saa716x-based card not used.
What I can see in the log: a lot of coherent mappings from sata_mv and orion_ehci, a few from mv643xx_eth, no other coherent mappings. All coherent mappings are page aligned, some of them (from orion_ehci) are not really small (as claimed in __alloc_from_pool).
I don't believe in a memory leak. When I restart vdr (the application utilizing the dvb sticks) then there is enough dma memory available again.
Regards, Soeren
Please find attached a debug log generated with your patch.
I used the sata disk and two em28xx dvb sticks, no other usb devices, no ethernet cable connected, tuners on saa716x-based card not used.
What I can see in the log: a lot of coherent mappings from sata_mv and orion_ehci, a few from mv643xx_eth, no other coherent mappings. All coherent mappings are page aligned, some of them (from orion_ehci) are not really small (as claimed in __alloc_from_pool).
I don't believe in a memory leak. When I restart vdr (the application utilizing the dvb sticks) then there is enough dma memory available again.
Hi Soeren
We should be able to rule out a leak. Mount debugfg and then:
while [ /bin/true ] ; do cat /debug/dma-api/num_free_entries ; sleep 60 ; done
while you are capturing. See if the number goes down.
Andrew
On 19.01.2013 19:59, Andrew Lunn wrote:
Please find attached a debug log generated with your patch.
I used the sata disk and two em28xx dvb sticks, no other usb devices, no ethernet cable connected, tuners on saa716x-based card not used.
What I can see in the log: a lot of coherent mappings from sata_mv and orion_ehci, a few from mv643xx_eth, no other coherent mappings. All coherent mappings are page aligned, some of them (from orion_ehci) are not really small (as claimed in __alloc_from_pool).
I don't believe in a memory leak. When I restart vdr (the application utilizing the dvb sticks) then there is enough dma memory available again.
Hi Soeren
We should be able to rule out a leak. Mount debugfg and then:
while [ /bin/true ] ; do cat /debug/dma-api/num_free_entries ; sleep 60 ; done
while you are capturing. See if the number goes down.
Andrew
Now I built a kernel with debugfs enabled. It is not clear to me what I can see from the dma-api/num_free_entries output. After reboot (vdr running) I see decreasing numbers (3453 3452 3445 3430...), min_free_entries is lower (3390). Sometimes the output is constant for several minutes ( 3396 3396 3396 3396 3396,...)
Soeren
On Wed, Jan 23, 2013 at 04:30:53PM +0100, Soeren Moch wrote:
On 19.01.2013 19:59, Andrew Lunn wrote:
Please find attached a debug log generated with your patch.
I used the sata disk and two em28xx dvb sticks, no other usb devices, no ethernet cable connected, tuners on saa716x-based card not used.
What I can see in the log: a lot of coherent mappings from sata_mv and orion_ehci, a few from mv643xx_eth, no other coherent mappings. All coherent mappings are page aligned, some of them (from orion_ehci) are not really small (as claimed in __alloc_from_pool).
I don't believe in a memory leak. When I restart vdr (the application utilizing the dvb sticks) then there is enough dma memory available again.
Hi Soeren
We should be able to rule out a leak. Mount debugfg and then:
while [ /bin/true ] ; do cat /debug/dma-api/num_free_entries ; sleep 60 ; done
while you are capturing. See if the number goes down.
AndrewNow I built a kernel with debugfs enabled. It is not clear to me what I can see from the dma-api/num_free_entries output. After reboot (vdr running) I see decreasing numbers (3453 3452 3445 3430...), min_free_entries is lower (3390). Sometimes the output is constant for several minutes ( 3396 3396 3396 3396 3396,...)
We are interesting in the long term behavior. Does it gradually go down? Or is it stable? If it goes down over time, its clearly a leak somewhere.
Andrew
On 23.01.2013 17:25, Andrew Lunn wrote:
On Wed, Jan 23, 2013 at 04:30:53PM +0100, Soeren Moch wrote:
On 19.01.2013 19:59, Andrew Lunn wrote:
Please find attached a debug log generated with your patch.
I used the sata disk and two em28xx dvb sticks, no other usb devices, no ethernet cable connected, tuners on saa716x-based card not used.
What I can see in the log: a lot of coherent mappings from sata_mv and orion_ehci, a few from mv643xx_eth, no other coherent mappings. All coherent mappings are page aligned, some of them (from orion_ehci) are not really small (as claimed in __alloc_from_pool).
I don't believe in a memory leak. When I restart vdr (the application utilizing the dvb sticks) then there is enough dma memory available again.
Hi Soeren
We should be able to rule out a leak. Mount debugfg and then:
while [ /bin/true ] ; do cat /debug/dma-api/num_free_entries ; sleep 60 ; done
while you are capturing. See if the number goes down.
AndrewNow I built a kernel with debugfs enabled. It is not clear to me what I can see from the dma-api/num_free_entries output. After reboot (vdr running) I see decreasing numbers (3453 3452 3445 3430...), min_free_entries is lower (3390). Sometimes the output is constant for several minutes ( 3396 3396 3396 3396 3396,...)
We are interesting in the long term behavior. Does it gradually go down? Or is it stable? If it goes down over time, its clearly a leak somewhere.
Now (in the last hour) stable, occasionally lower numbers: 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3365 3396 3394 3396 3396 3396 3396 3373 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3353 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3394 3396 3396 3396 3396 3396 3396 3396
Before the last pool exhaustion going down: 3395 3395 3389 3379 3379 3374 3367 3360 3352 3343 3343 3343 3342 3336 3332 3324 3318 3314 3310 3307 3305 3299 3290 3283 3279 3272 3266 3265 3247 3247 3247 3242 3236 3236
Soeren
On 23.01.2013 18:07, Soeren Moch wrote:
On 23.01.2013 17:25, Andrew Lunn wrote:
On Wed, Jan 23, 2013 at 04:30:53PM +0100, Soeren Moch wrote:
On 19.01.2013 19:59, Andrew Lunn wrote:
Please find attached a debug log generated with your patch.
I used the sata disk and two em28xx dvb sticks, no other usb devices, no ethernet cable connected, tuners on saa716x-based card not used.
What I can see in the log: a lot of coherent mappings from sata_mv and orion_ehci, a few from mv643xx_eth, no other coherent mappings. All coherent mappings are page aligned, some of them (from orion_ehci) are not really small (as claimed in __alloc_from_pool).
I don't believe in a memory leak. When I restart vdr (the application utilizing the dvb sticks) then there is enough dma memory available again.
Hi Soeren
We should be able to rule out a leak. Mount debugfg and then:
while [ /bin/true ] ; do cat /debug/dma-api/num_free_entries ; sleep 60 ; done
while you are capturing. See if the number goes down.
AndrewNow I built a kernel with debugfs enabled. It is not clear to me what I can see from the dma-api/num_free_entries output. After reboot (vdr running) I see decreasing numbers (3453 3452 3445 3430...), min_free_entries is lower (3390). Sometimes the output is constant for several minutes ( 3396 3396 3396 3396 3396,...)
We are interesting in the long term behavior. Does it gradually go down? Or is it stable? If it goes down over time, its clearly a leak somewhere.
Now (in the last hour) stable, occasionally lower numbers: 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3365 3396 3394 3396 3396 3396 3396 3373 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3353 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3394 3396 3396 3396 3396 3396 3396 3396
Before the last pool exhaustion going down: 3395 3395 3389 3379 3379 3374 3367 3360 3352 3343 3343 3343 3342 3336 3332 3324 3318 3314 3310 3307 3305 3299 3290 3283 3279 3272 3266 3265 3247 3247 3247 3242 3236 3236
Here I stopped vdr (and so closed all dvb_demux devices), the number was remaining the same 3236, even after restart of vdr (and restart of streaming).
Soeren
Now (in the last hour) stable, occasionally lower numbers: 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3365 3396 3394 3396 3396 3396 3396 3373 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3353 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3394 3396 3396 3396 3396 3396 3396 3396
Before the last pool exhaustion going down: 3395 3395 3389 3379 3379 3374 3367 3360 3352 3343 3343 3343 3342 3336 3332 3324 3318 3314 3310 3307 3305 3299 3290 3283 3279 3272 3266 3265 3247 3247 3247 3242 3236 3236
Here I stopped vdr (and so closed all dvb_demux devices), the number was remaining the same 3236, even after restart of vdr (and restart of streaming).
So it does suggest a leak. Probably somewhere on an error path, e.g. its lost video sync.
Andrew
On 23.01.2013 19:10, Andrew Lunn wrote:
Now (in the last hour) stable, occasionally lower numbers: 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3365 3396 3394 3396 3396 3396 3396 3373 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3353 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3394 3396 3396 3396 3396 3396 3396 3396
Before the last pool exhaustion going down: 3395 3395 3389 3379 3379 3374 3367 3360 3352 3343 3343 3343 3342 3336 3332 3324 3318 3314 3310 3307 3305 3299 3290 3283 3279 3272 3266 3265 3247 3247 3247 3242 3236 3236
Here I stopped vdr (and so closed all dvb_demux devices), the number was remaining the same 3236, even after restart of vdr (and restart of streaming).
So it does suggest a leak. Probably somewhere on an error path, e.g. its lost video sync.
Now I activated the debug messages in em28xx. From the messages I see no correlation of the pool exhaustion and lost sync. Also I cannot see any error messages from the em28xx driver. I see a lot of init_isoc/stop_urbs (maybe EPG scan?) without draining the coherent pool (checked with 'cat /debug/dma-api/num_free_entries', which gave stable numbers), but after half an hour there are only init_isoc messages without corresponding stop_urbs messages and num_free_entries decreased until coherent pool exhaustion.
Any idea where the memory leak is? What is allocating coherent buffers for orion-ehci?
Soeren
Jan 28 20:46:03 guruvdr kernel: em28xx #0/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:03 guruvdr kernel: em28xx #0 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:03 guruvdr kernel: em28xx #1/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:03 guruvdr kernel: em28xx #1 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:23 guruvdr kernel: em28xx #0 em28xx_stop_urbs :em28xx: called em28xx_stop_urbs Jan 28 20:46:23 guruvdr kernel: em28xx #1 em28xx_stop_urbs :em28xx: called em28xx_stop_urbs Jan 28 20:46:24 guruvdr kernel: em28xx #0/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:24 guruvdr kernel: em28xx #0 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:24 guruvdr kernel: em28xx #1/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:24 guruvdr kernel: em28xx #1 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:44 guruvdr kernel: em28xx #1 em28xx_stop_urbs :em28xx: called em28xx_stop_urbs Jan 28 20:46:44 guruvdr kernel: em28xx #0 em28xx_stop_urbs :em28xx: called em28xx_stop_urbs Jan 28 20:46:45 guruvdr kernel: em28xx #1/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:45 guruvdr kernel: em28xx #1 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:45 guruvdr kernel: em28xx #0/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:45 guruvdr kernel: em28xx #0 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:54:33 guruvdr kernel: ERROR: 1024 KiB atomic DMA coherent pool is too small! Jan 28 20:54:33 guruvdr kernel: Please increase it with coherent_pool= kernel parameter!
On Mon, Jan 28, 2013 at 09:59:18PM +0100, Soeren Moch wrote:
On 23.01.2013 19:10, Andrew Lunn wrote:
Now (in the last hour) stable, occasionally lower numbers: 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3365 3396 3394 3396 3396 3396 3396 3373 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3353 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3396 3394 3396 3396 3396 3396 3396 3396 3396
Before the last pool exhaustion going down: 3395 3395 3389 3379 3379 3374 3367 3360 3352 3343 3343 3343 3342 3336 3332 3324 3318 3314 3310 3307 3305 3299 3290 3283 3279 3272 3266 3265 3247 3247 3247 3242 3236 3236
Here I stopped vdr (and so closed all dvb_demux devices), the number was remaining the same 3236, even after restart of vdr (and restart of streaming).
So it does suggest a leak. Probably somewhere on an error path, e.g. its lost video sync.
Now I activated the debug messages in em28xx. From the messages I see no correlation of the pool exhaustion and lost sync. Also I cannot see any error messages from the em28xx driver. I see a lot of init_isoc/stop_urbs (maybe EPG scan?) without draining the coherent pool (checked with 'cat /debug/dma-api/num_free_entries', which gave stable numbers), but after half an hour there are only init_isoc messages without corresponding stop_urbs messages and num_free_entries decreased until coherent pool exhaustion.
Any idea where the memory leak is? What is allocating coherent buffers for orion-ehci?
Keeping in mind that I am completely unfamiliar with usb dvb, my best guess is that the problem is in em28xx-core.c:1131
According to your log messages, it is in mode 2, which is EM28XX_DIGITAL_MODE.
There seem to be good hints in
86d38d1e [media] em28xx: pre-allocate DVB isoc transfer buffers
I added the relevant parties to the To:...
For Gianluca and Mauro, the whole thread may be found at:
http://markmail.org/message/wm4wlgzoudixd4so#query:+page:1+mid:o7phz7cosmwpc...
thx,
Jason.
Soeren
Jan 28 20:46:03 guruvdr kernel: em28xx #0/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:03 guruvdr kernel: em28xx #0 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:03 guruvdr kernel: em28xx #1/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:03 guruvdr kernel: em28xx #1 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:23 guruvdr kernel: em28xx #0 em28xx_stop_urbs :em28xx: called em28xx_stop_urbs Jan 28 20:46:23 guruvdr kernel: em28xx #1 em28xx_stop_urbs :em28xx: called em28xx_stop_urbs Jan 28 20:46:24 guruvdr kernel: em28xx #0/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:24 guruvdr kernel: em28xx #0 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:24 guruvdr kernel: em28xx #1/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:24 guruvdr kernel: em28xx #1 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:44 guruvdr kernel: em28xx #1 em28xx_stop_urbs :em28xx: called em28xx_stop_urbs Jan 28 20:46:44 guruvdr kernel: em28xx #0 em28xx_stop_urbs :em28xx: called em28xx_stop_urbs Jan 28 20:46:45 guruvdr kernel: em28xx #1/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:45 guruvdr kernel: em28xx #1 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:46:45 guruvdr kernel: em28xx #0/2-dvb: Using 5 buffers each with 64 x 940 bytes Jan 28 20:46:45 guruvdr kernel: em28xx #0 em28xx_init_isoc :em28xx: called em28xx_init_isoc in mode 2 Jan 28 20:54:33 guruvdr kernel: ERROR: 1024 KiB atomic DMA coherent pool is too small! Jan 28 20:54:33 guruvdr kernel: Please increase it with coherent_pool= kernel parameter!
Now I activated the debug messages in em28xx. From the messages I see no correlation of the pool exhaustion and lost sync. Also I cannot see any error messages from the em28xx driver. I see a lot of init_isoc/stop_urbs (maybe EPG scan?) without draining the coherent pool (checked with 'cat /debug/dma-api/num_free_entries', which gave stable numbers), but after half an hour there are only init_isoc messages without corresponding stop_urbs messages and num_free_entries decreased until coherent pool exhaustion.
Hi Soeren
em28xx_stop_urbs() is only called by em28xx_stop_streaming().
em28xx_stop_streaming() is only called by em28xx_stop_feed() when 0 == dvb->nfeeds.
em28xx_stop_feed()and em28xx_start_feed() look O.K, dvb->nfeeds is protected by a mutex etc.
Now, em28xx_init_isoc() is also called by buffer_prepare(). This uses em28xx_alloc_isoc() to do the actual allocation, and that function sets up the urb such that on completion the function em28xx_irq_callback() is called.
It looks like there might be issues here:
Once the data has been copied out, it resubmits the urb:
urb->status = usb_submit_urb(urb, GFP_ATOMIC); if (urb->status) { em28xx_isocdbg("urb resubmit failed (error=%i)\n", urb->status); }
However, if the ubs_submit_urb fails, it looks like the urb is lost.
If you look at other code submitting urbs you have this pattern:
rc = usb_submit_urb(isoc_bufs->urb[i], GFP_ATOMIC); if (rc) { em28xx_err("submit of urb %i failed (error=%i)\n", i, rc); em28xx_uninit_isoc(dev, mode); return rc; }
Do you have your build such that you would see "urb resubmit failed" in your logs? Are there any?
Andrew
On 29.01.2013 12:02, Andrew Lunn wrote:
Now I activated the debug messages in em28xx. From the messages I see no correlation of the pool exhaustion and lost sync. Also I cannot see any error messages from the em28xx driver. I see a lot of init_isoc/stop_urbs (maybe EPG scan?) without draining the coherent pool (checked with 'cat /debug/dma-api/num_free_entries', which gave stable numbers), but after half an hour there are only init_isoc messages without corresponding stop_urbs messages and num_free_entries decreased until coherent pool exhaustion.
Hi Soeren
em28xx_stop_urbs() is only called by em28xx_stop_streaming().
em28xx_stop_streaming() is only called by em28xx_stop_feed() when 0 == dvb->nfeeds.
em28xx_stop_feed()and em28xx_start_feed() look O.K, dvb->nfeeds is protected by a mutex etc.
Now, em28xx_init_isoc() is also called by buffer_prepare(). This uses em28xx_alloc_isoc() to do the actual allocation, and that function sets up the urb such that on completion the function em28xx_irq_callback() is called.
It looks like there might be issues here:
Once the data has been copied out, it resubmits the urb:
urb->status = usb_submit_urb(urb, GFP_ATOMIC); if (urb->status) { em28xx_isocdbg("urb resubmit failed (error=%i)\n", urb->status); }However, if the ubs_submit_urb fails, it looks like the urb is lost.
If you look at other code submitting urbs you have this pattern:
rc = usb_submit_urb(isoc_bufs->urb[i], GFP_ATOMIC); if (rc) { em28xx_err("submit of urb %i failed (error=%i)\n", i, rc); em28xx_uninit_isoc(dev, mode); return rc; }Do you have your build such that you would see "urb resubmit failed" in your logs? Are there any?
I only had "urb resubmit failed" messages _after_ the coherent pool exhaustion. So I guess something below the usb_submit_urb call is allocating (too much) memory, sometimes. Or can dvb_demux allocate memory and blame orion-ehci for it?
Soeren
On Saturday 19 January 2013, Soeren Moch wrote:
What I can see in the log: a lot of coherent mappings from sata_mv and orion_ehci, a few from mv643xx_eth, no other coherent mappings. All coherent mappings are page aligned, some of them (from orion_ehci) are not really small (as claimed in __alloc_from_pool).
Right. Unfortunately, the output does not show which of the mappings are atomic, so we still need to look through those that can be atomic to understand what's going on. There are a few megabytes of coherent mappings in total according to the output, but it seems that a significant portion of them is atomic, which is a bit unexpected.
I don't believe in a memory leak. When I restart vdr (the application utilizing the dvb sticks) then there is enough dma memory available again.
I found at least one source line that incorrectly uses an atomic allocation, in ehci_mem_init():
dma_alloc_coherent (ehci_to_hcd(ehci)->self.controller, ehci->periodic_size * sizeof(__le32), &ehci->periodic_dma, 0);
The last argument is the GFP_ flag, which should never be zero, as that is implicit !wait. This function is called only once, so it is not the actual culprit, but there could be other instances where we accidentally allocate something as GFP_ATOMIC.
The total number of allocations I found for each type are
sata_mv: 66 pages (270336 bytes) mv643xx_eth: 4 pages == (16384 bytes) orion_ehci: 154 pages (630784 bytes) orion_ehci (atomic): 256 pages (1048576 bytes)
from the distribution of the numbers, it seems that there is exactly 1 MB of data allocated between bus addresses 0x1f90000 and 0x1f9ffff, allocated in individual pages. This matches the size of your pool, so it's definitely something coming from USB, and no single other allocation, but it does not directly point to a specific line of code.
One thing I found was that the ARM dma-mapping code seems buggy in the way that it does a bitwise and between the gfp mask and GFP_ATOMIC, which does not work because GFP_ATOMIC is defined by the absence of __GFP_WAIT.
I believe we need the patch below, but it is not clear to me if that issue is related to your problem or now.
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6b2fb87..c57975f 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -640,7 +641,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
if (is_coherent || nommu()) addr = __alloc_simple_buffer(dev, size, gfp, &page); - else if (gfp & GFP_ATOMIC) + else if (!(gfp & __GFP_WAIT)) addr = __alloc_from_pool(size, &page); else if (!IS_ENABLED(CONFIG_CMA)) addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller); @@ -1272,7 +1273,7 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, *handle = DMA_ERROR_CODE; size = PAGE_ALIGN(size);
- if (gfp & GFP_ATOMIC) + if (!(gfp & __GFP_WAIT)) return __iommu_alloc_atomic(dev, size, handle);
pages = __iommu_alloc_buffer(dev, size, gfp, attrs); 8<-------
There is one more code path I could find, which is usb_submit_urb() => usb_hcd_submit_urb => ehci_urb_enqueue() => submit_async() => qh_append_tds() => qh_make(GFP_ATOMIC) => ehci_qh_alloc() => dma_pool_alloc() => pool_alloc_page() => dma_alloc_coherent()
So even for a GFP_KERNEL passed into usb_submit_urb, the ehci driver causes the low-level allocation to be GFP_ATOMIC, because qh_append_tds() is called under a spinlock. If we have hundreds of URBs in flight, that will exhaust the pool rather quickly.
Arnd
On 01/19/13 21:05, Arnd Bergmann wrote:
I found at least one source line that incorrectly uses an atomic allocation, in ehci_mem_init():
dma_alloc_coherent (ehci_to_hcd(ehci)->self.controller, ehci->periodic_size * sizeof(__le32), &ehci->periodic_dma, 0);The last argument is the GFP_ flag, which should never be zero, as that is implicit !wait. This function is called only once, so it is not the actual culprit, but there could be other instances where we accidentally allocate something as GFP_ATOMIC.
The total number of allocations I found for each type are
sata_mv: 66 pages (270336 bytes) mv643xx_eth: 4 pages == (16384 bytes) orion_ehci: 154 pages (630784 bytes) orion_ehci (atomic): 256 pages (1048576 bytes)
from the distribution of the numbers, it seems that there is exactly 1 MB of data allocated between bus addresses 0x1f90000 and 0x1f9ffff, allocated in individual pages. This matches the size of your pool, so it's definitely something coming from USB, and no single other allocation, but it does not directly point to a specific line of code.
Very interesting, so this is no fragmentation problem nor something caused by sata or ethernet.
One thing I found was that the ARM dma-mapping code seems buggy in the way that it does a bitwise and between the gfp mask and GFP_ATOMIC, which does not work because GFP_ATOMIC is defined by the absence of __GFP_WAIT.
I believe we need the patch below, but it is not clear to me if that issue is related to your problem or now.
Out of curiosity I checked include/linux/gfp.h. GFP_ATOMIC is defined as __GFP_HIGH (which means 'use emergency pool', and no wait), so this patch should not make any difference for "normal" (GPF_ATOMIC / GFP_KERNEL) allocations, only for gfp_flags accidentally set to zero. So, can a new test with this patch help to debug the pool exhaustion?
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6b2fb87..c57975f 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -640,7 +641,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, if (is_coherent || nommu()) addr = __alloc_simple_buffer(dev, size, gfp, &page);
- else if (gfp & GFP_ATOMIC)
- else if (!(gfp & __GFP_WAIT)) addr = __alloc_from_pool(size, &page); else if (!IS_ENABLED(CONFIG_CMA)) addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller);
@@ -1272,7 +1273,7 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, *handle = DMA_ERROR_CODE; size = PAGE_ALIGN(size);
- if (gfp & GFP_ATOMIC)
- if (!(gfp & __GFP_WAIT)) return __iommu_alloc_atomic(dev, size, handle);
pages = __iommu_alloc_buffer(dev, size, gfp, attrs); 8<-------
There is one more code path I could find, which is usb_submit_urb() => usb_hcd_submit_urb => ehci_urb_enqueue() => submit_async() => qh_append_tds() => qh_make(GFP_ATOMIC) => ehci_qh_alloc() => dma_pool_alloc() => pool_alloc_page() => dma_alloc_coherent()
So even for a GFP_KERNEL passed into usb_submit_urb, the ehci driver causes the low-level allocation to be GFP_ATOMIC, because qh_append_tds() is called under a spinlock. If we have hundreds of URBs in flight, that will exhaust the pool rather quickly.
Maybe there are hundreds of URBs in flight in my application, I have no idea how to check this. It seems to me that bad reception conditions (lost lock / regained lock messages for some dvb channels) accelerate the buffer exhaustion. But even with a 4MB coherent pool I see the error. Is there any chance to fix this in the usb or dvb subsystem (or wherever)? Should I try to further increase the pool size, or what else can I do besides using an older kernel?
Soeren
On Monday 21 January 2013, Soeren Moch wrote:
On 01/19/13 21:05, Arnd Bergmann wrote:
from the distribution of the numbers, it seems that there is exactly 1 MB of data allocated between bus addresses 0x1f90000 and 0x1f9ffff, allocated in individual pages. This matches the size of your pool, so it's definitely something coming from USB, and no single other allocation, but it does not directly point to a specific line of code.
Very interesting, so this is no fragmentation problem nor something caused by sata or ethernet.
Right.
One thing I found was that the ARM dma-mapping code seems buggy in the way that it does a bitwise and between the gfp mask and GFP_ATOMIC, which does not work because GFP_ATOMIC is defined by the absence of __GFP_WAIT.
I believe we need the patch below, but it is not clear to me if that issue is related to your problem or now.
Out of curiosity I checked include/linux/gfp.h. GFP_ATOMIC is defined as __GFP_HIGH (which means 'use emergency pool', and no wait), so this patch should not make any difference for "normal" (GPF_ATOMIC / GFP_KERNEL) allocations, only for gfp_flags accidentally set to zero.
Yes, or one of the rare cases where someone intentionally does something like (GFP_ATOMIC & !__GFP_HIGH) or (GFP_KERNEL || __GFP_HIGH), which are both wrong.
So, can a new test with this patch help to debug the pool exhaustion?
Yes, but I would not expect this to change much. It's a bug, but not likely the one you are hitting.
So even for a GFP_KERNEL passed into usb_submit_urb, the ehci driver causes the low-level allocation to be GFP_ATOMIC, because qh_append_tds() is called under a spinlock. If we have hundreds of URBs in flight, that will exhaust the pool rather quickly.
Maybe there are hundreds of URBs in flight in my application, I have no idea how to check this.
I don't know a lot about USB, but I always assumed that this was not a normal condition and that there are only a couple of URBs per endpoint used at a time. Maybe Greg or someone else with a USB background can shed some light on this.
It seems to me that bad reception conditions (lost lock / regained lock messages for some dvb channels) accelerate the buffer exhaustion. But even with a 4MB coherent pool I see the error. Is there any chance to fix this in the usb or dvb subsystem (or wherever)? Should I try to further increase the pool size, or what else can I do besides using an older kernel?
There are two things that I think can be done if hundreds of URBs is indeed the normal working condition for this driver:
* change the locking in your driver so it can actually call usb_submit_urb using GFP_KERNEL rather than GFP_ATOMIC * after that is done, rework the ehci_hcd driver so it can do the allocation inside of the submit_urb path to use the mem_flags rather than unconditional GFP_ATOMIC.
Note that the problem you are seeing does not just exist in the case of the atomic coherent pool getting exhausted, but also on any platform that runs into an out-of-memory condition.
Arnd
On Mon, Jan 21, 2013 at 06:55:25PM +0000, Arnd Bergmann wrote:
On Monday 21 January 2013, Soeren Moch wrote:
On 01/19/13 21:05, Arnd Bergmann wrote:
from the distribution of the numbers, it seems that there is exactly 1 MB of data allocated between bus addresses 0x1f90000 and 0x1f9ffff, allocated in individual pages. This matches the size of your pool, so it's definitely something coming from USB, and no single other allocation, but it does not directly point to a specific line of code.
Very interesting, so this is no fragmentation problem nor something caused by sata or ethernet.
Right.
One thing I found was that the ARM dma-mapping code seems buggy in the way that it does a bitwise and between the gfp mask and GFP_ATOMIC, which does not work because GFP_ATOMIC is defined by the absence of __GFP_WAIT.
I believe we need the patch below, but it is not clear to me if that issue is related to your problem or now.
Out of curiosity I checked include/linux/gfp.h. GFP_ATOMIC is defined as __GFP_HIGH (which means 'use emergency pool', and no wait), so this patch should not make any difference for "normal" (GPF_ATOMIC / GFP_KERNEL) allocations, only for gfp_flags accidentally set to zero.
Yes, or one of the rare cases where someone intentionally does something like (GFP_ATOMIC & !__GFP_HIGH) or (GFP_KERNEL || __GFP_HIGH), which are both wrong.
So, can a new test with this patch help to debug the pool exhaustion?
Yes, but I would not expect this to change much. It's a bug, but not likely the one you are hitting.
So even for a GFP_KERNEL passed into usb_submit_urb, the ehci driver causes the low-level allocation to be GFP_ATOMIC, because qh_append_tds() is called under a spinlock. If we have hundreds of URBs in flight, that will exhaust the pool rather quickly.
Maybe there are hundreds of URBs in flight in my application, I have no idea how to check this.
I don't know a lot about USB, but I always assumed that this was not a normal condition and that there are only a couple of URBs per endpoint used at a time. Maybe Greg or someone else with a USB background can shed some light on this.
There's no restriction on how many URBs a driver can have outstanding at once, and if you have a system with a lot of USB devices running at the same time, there could be lots of URBs in flight depending on the number of host controllers and devices and drivers being used.
Sorry,
greg k-h
On Monday 21 January 2013, Greg KH wrote:
I don't know a lot about USB, but I always assumed that this was not a normal condition and that there are only a couple of URBs per endpoint used at a time. Maybe Greg or someone else with a USB background can shed some light on this.
There's no restriction on how many URBs a driver can have outstanding at once, and if you have a system with a lot of USB devices running at the same time, there could be lots of URBs in flight depending on the number of host controllers and devices and drivers being used.
Ok, thanks for clarifying that. I read some more of the em28xx driver, and while it does have a bunch of URBs in flight, there are only five audio and five video URBs that I see simultaneously being submitted, and then resubmitted from their completion handlers. I think this means that there should be 10 URBs active at any given time in this driver, which does not explain why we get 256 allocations.
I also noticed that the initial submissions are all atomic but don't need to, so it may be worth trying the patch below, which should also help in low-memory situations. We could also try moving the resubmission into a workqueue in order to let those be GFP_KERNEL, but I don't think that will help.
Arnd
diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c index 2fdb66e..8b789f4 100644 --- a/drivers/media/usb/em28xx/em28xx-audio.c +++ b/drivers/media/usb/em28xx/em28xx-audio.c @@ -177,12 +177,12 @@ static int em28xx_init_audio_isoc(struct em28xx *dev) struct urb *urb; int j, k;
- dev->adev.transfer_buffer[i] = kmalloc(sb_size, GFP_ATOMIC); + dev->adev.transfer_buffer[i] = kmalloc(sb_size, GFP_KERNEL); if (!dev->adev.transfer_buffer[i]) return -ENOMEM;
memset(dev->adev.transfer_buffer[i], 0x80, sb_size); - urb = usb_alloc_urb(EM28XX_NUM_AUDIO_PACKETS, GFP_ATOMIC); + urb = usb_alloc_urb(EM28XX_NUM_AUDIO_PACKETS, GFP_KERNEL); if (!urb) { em28xx_errdev("usb_alloc_urb failed!\n"); for (j = 0; j < i; j++) { @@ -212,7 +212,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev) }
for (i = 0; i < EM28XX_AUDIO_BUFS; i++) { - errCode = usb_submit_urb(dev->adev.urb[i], GFP_ATOMIC); + errCode = usb_submit_urb(dev->adev.urb[i], GFP_KERNEL); if (errCode) { em28xx_errdev("submit of audio urb failed\n"); em28xx_deinit_isoc_audio(dev); diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c index bed07a6..c5a2c4b 100644 --- a/drivers/media/usb/em28xx/em28xx-core.c +++ b/drivers/media/usb/em28xx/em28xx-core.c @@ -1166,7 +1166,7 @@ int em28xx_init_isoc(struct em28xx *dev, enum em28xx_mode mode,
/* submit urbs and enables IRQ */ for (i = 0; i < isoc_bufs->num_bufs; i++) { - rc = usb_submit_urb(isoc_bufs->urb[i], GFP_ATOMIC); + rc = usb_submit_urb(isoc_bufs->urb[i], GFP_KERNEL); if (rc) { em28xx_err("submit of urb %i failed (error=%i)\n", i, rc);
On 22.01.2013 19:13, Arnd Bergmann wrote:
On Monday 21 January 2013, Greg KH wrote:
I don't know a lot about USB, but I always assumed that this was not a normal condition and that there are only a couple of URBs per endpoint used at a time. Maybe Greg or someone else with a USB background can shed some light on this.
There's no restriction on how many URBs a driver can have outstanding at once, and if you have a system with a lot of USB devices running at the same time, there could be lots of URBs in flight depending on the number of host controllers and devices and drivers being used.
I only use one host controller and (in this test) two usb devices with the same driver.
Ok, thanks for clarifying that. I read some more of the em28xx driver, and while it does have a bunch of URBs in flight, there are only five audio and five video URBs that I see simultaneously being submitted, and then resubmitted from their completion handlers. I think this means that there should be 10 URBs active at any given time in this driver, which does not explain why we get 256 allocations.
I think the audio part of the em28xx bridge is not used in my DVB tests.
Are there other allocations from orion-ehci directly? Maybe something special for isochronous transfers (since there is no problem with my other dvb sticks using bulk transfers)?
I also noticed that the initial submissions are all atomic but don't need to, so it may be worth trying the patch below, which should also help in low-memory situations. We could also try moving the resubmission into a workqueue in order to let those be GFP_KERNEL, but I don't think that will help.
I built a linux-3.7.4 with the em28xx patch and both of your dma-mapping.c patches. I still see the ERROR: 1024 KiB atomic DMA coherent pool is too small!
Soeren
On Thu, Jan 17, 2013 at 08:26:45PM +0000, Arnd Bergmann wrote:
On Thursday 17 January 2013, Soeren Moch wrote:
On 17.01.2013 11:49, Arnd Bergmann wrote:
On Wednesday 16 January 2013, Soeren Moch wrote:
I will see what I can do here. Is there an easy way to track the buffer usage without having to wait for complete exhaustion?
DMA_API_DEBUG
OK, maybe I can try this.
I tried this. Not what i expected. We have at least one problem with the ethernet driver:
WARNING: at lib/dma-debug.c:933 check_unmap+0x4b8/0x8a8() mv643xx_eth_port mv643xx_eth_port.0: DMA-API: device driver failed to check map error[device address=0x000000001f22be00] [size=1536 bytes] [mapped as single] Modules linked in: [<c000db10>] (unwind_backtrace+0x0/0xf4) from [<c0016c44>] (warn_slowpath_common+0x4c/0x64) [<c0016c44>] (warn_slowpath_common+0x4c/0x64) from [<c0016cf0>] (warn_slowpath_fmt+0x30/0x40) [<c0016cf0>] (warn_slowpath_fmt+0x30/0x40) from [<c01ab540>] (check_unmap+0x4b8/0x8a8) [<c01ab540>] (check_unmap+0x4b8/0x8a8) from [<c01abbb8>] (debug_dma_unmap_page+0x8c/0x98) [<c01abbb8>] (debug_dma_unmap_page+0x8c/0x98) from [<c025cb1c>] (mv643xx_eth_poll+0x630/0x800) [<c025cb1c>] (mv643xx_eth_poll+0x630/0x800) from [<c0331d9c>] (net_rx_action+0xcc/0x1d4) [<c0331d9c>] (net_rx_action+0xcc/0x1d4) from [<c001e1b0>] (__do_softirq+0xa8/0x170) [<c001e1b0>] (__do_softirq+0xa8/0x170) from [<c001e3e8>] (do_softirq+0x5c/0x6c) [<c001e3e8>] (do_softirq+0x5c/0x6c) from [<c001e610>] (local_bh_enable+0xcc/0xdc) [<c001e610>] (local_bh_enable+0xcc/0xdc) from [<c0359c74>] (ip_finish_output+0x1c8/0x39c) [<c0359c74>] (ip_finish_output+0x1c8/0x39c) from [<c03571a4>] (ip_local_out+0x28/0x2c) [<c03571a4>] (ip_local_out+0x28/0x2c) from [<c0359564>] (ip_queue_xmit+0x118/0x338) [<c0359564>] (ip_queue_xmit+0x118/0x338) from [<c036e58c>] (tcp_transmit_skb+0x3fc/0x8e4) [<c036e58c>] (tcp_transmit_skb+0x3fc/0x8e4) from [<c0371218>] (tcp_write_xmit+0x228/0xb08) [<c0371218>] (tcp_write_xmit+0x228/0xb08) from [<c0371b6c>] (__tcp_push_pending_frames+0x30/0x9c) [<c0371b6c>] (__tcp_push_pending_frames+0x30/0x9c) from [<c0362940>] (tcp_sendmsg+0x158/0xdc4) [<c0362940>] (tcp_sendmsg+0x158/0xdc4) from [<c0386620>] (inet_sendmsg+0x38/0x74) [<c0386620>] (inet_sendmsg+0x38/0x74) from [<c031f370>] (sock_aio_write+0x12c/0x138) [<c031f370>] (sock_aio_write+0x12c/0x138) from [<c00a62f4>] (do_sync_write+0xa0/0xd0) [<c00a62f4>] (do_sync_write+0xa0/0xd0) from [<c00a6f18>] (vfs_write+0x13c/0x144) [<c00a6f18>] (vfs_write+0x13c/0x144) from [<c00a6ff0>] (sys_write+0x44/0x70) [<c00a6ff0>] (sys_write+0x44/0x70) from [<c0008ce0>] (ret_fast_syscall+0x0/0x2c) ---[ end trace b75faa8779652e63 ]---
I'm getting about 4 errors reported a second from the ethernet driver.
Before i look at issues with em28xx i will first try to get the noise from the ethernet driver sorted out.
Andrew
On 01/15/2013 05:56 PM, Jason Cooper wrote:
Greg,
I've added you to the this thread hoping for a little insight into USB drivers and their use of coherent and GFP_ATOMIC. Am I barking up the wrong tree by looking a the drivers?
On Mon, Jan 14, 2013 at 12:56:57PM +0100, Soeren Moch wrote:
On 20.11.2012 15:31, 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. Additionaly, on ARM architecture any driver which is using dmapools will sooner or later trigger the following error: "ERROR: 256 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!". Increasing the coherent pool size usually doesn't help much and only delays such error, because all GFP_ATOMIC DMA allocations are always served from the special, very limited memory pool.
This patch changes the dmapool code to correctly use gfp flags provided by the dmapool caller.
Reported-by: Soeren Mochsmoch@web.de Reported-by: Thomas Petazzonithomas.petazzoni@free-electrons.com Signed-off-by: Marek Szyprowskim.szyprowski@samsung.com Tested-by: Andrew Lunnandrew@lunn.ch Tested-by: Soeren Mochsmoch@web.de
Now I tested linux-3.7.1 (this patch is included there) on my Marvell Kirkwood system. I still see
ERROR: 1024 KiB atomic DMA coherent pool is too small! Please increase it with coherent_pool= kernel parameter!
after several hours of runtime under heavy load with SATA and DVB-Sticks (em28xx / drxk and dib0700).
Could you try running the system w/o the em28xx stick and see how it goes with v3.7.1?
Jason,
can you point out what you think we should be looking for?
I grep'd for 'GFP_' in drivers/media/usb and especially for dvb-usb (dib0700) it looks like most of the buffers in usb-urb.c are allocated GFP_ATOMIC. em28xx also allocates some of the buffers atomic.
If we look for a mem leak in one of the above drivers (including sata_mv), is there an easy way to keep track of allocated and freed kernel memory?
Sebastian
On Tue, Jan 15, 2013 at 09:05:50PM +0100, Sebastian Hesselbarth wrote:
If we look for a mem leak in one of the above drivers (including sata_mv), is there an easy way to keep track of allocated and freed kernel memory?
I'm inclined to think sata_mv is not the cause here, as there are many heavy users of it without error reports. The only thing different here are the three usb dvb dongles.
thx,
Jason.
linaro-mm-sig@lists.linaro.org
-
Andrew Lunn -
Andrew Morton -
Arnd Bergmann -
Greg KH -
Gregory CLEMENT -
Jason Cooper -
Marek Szyprowski -
Sebastian Hesselbarth -
Soeren Moch