linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Soeren Moch <smoch@web.de>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Jason Cooper <jason@lakedaemon.net>,
	Greg KH <gregkh@linuxfoundation.org>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Andrew Lunn <andrew@lunn.ch>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	linux-kernel@vger.kernel.org, Michal Hocko <mhocko@suse.cz>,
	linux-mm@kvack.org, Kyungmin Park <kyungmin.park@samsung.com>,
	Mel Gorman <mgorman@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	linaro-mm-sig@lists.linaro.org,
	linux-arm-kernel@lists.infradead.org,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH v2] mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls
Date: Mon, 21 Jan 2013 16:01:24 +0100	[thread overview]
Message-ID: <50FD5844.1010201@web.de> (raw)
In-Reply-To: <201301192005.20093.arnd@arndb.de>

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

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

  reply	other threads:[~2013-01-21 15:03 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-08  6:38 [PATCH] mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls Marek Szyprowski
2012-11-11 17:22 ` Andrew Lunn
2012-11-12  9:48   ` Soeren Moch
2012-11-12 10:38     ` Andrew Lunn
2012-11-12 11:03       ` Soeren Moch
2012-11-19  0:18 ` Jason Cooper
2012-11-19 22:48   ` Andrew Morton
2012-11-20 10:48     ` Marek Szyprowski
2012-11-20 19:52       ` Andrew Morton
2012-11-20 14:31     ` [PATCH v2] " Marek Szyprowski
2012-11-20 19:33       ` Andrew Morton
2012-11-20 20:27         ` Jason Cooper
2012-11-21  8:08         ` Marek Szyprowski
2012-11-21  8:36           ` Andrew Morton
2012-11-21  9:20             ` Marek Szyprowski
2012-11-21 19:17               ` Andrew Morton
2012-11-22 12:55                 ` Marek Szyprowski
2013-01-14 11:56       ` Soeren Moch
2013-01-15 16:56         ` Jason Cooper
2013-01-15 17:50           ` Greg KH
2013-01-15 20:16             ` Jason Cooper
2013-01-15 21:56               ` Jason Cooper
2013-01-16  0:17                 ` Soeren Moch
2013-01-16  2:40                   ` Jason Cooper
2013-01-16  3:24                     ` Soeren Moch
2013-01-16  8:55                       ` Soeren Moch
2013-01-16 15:50                         ` [PATCH] ata: sata_mv: fix sg_tbl_pool alignment Jason Cooper
2013-01-16 17:05                           ` Soeren Moch
2013-01-16 17:52                             ` Jason Cooper
2013-01-16 18:35                               ` Jason Cooper
2013-01-16 22:26                                 ` Soeren Moch
2013-01-16 23:10                               ` Soeren Moch
2013-01-17  9:11                               ` Soeren Moch
2013-01-16 17:32                         ` [PATCH v2] mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls Soeren Moch
2013-01-16 17:47                           ` Jason Cooper
2013-01-16 22:36                             ` Soeren Moch
2013-01-17 10:49                       ` Arnd Bergmann
2013-01-17 13:47                         ` Soeren Moch
2013-01-17 20:26                           ` Arnd Bergmann
2013-01-19 15:29                             ` Soeren Moch
2013-01-19 18:59                               ` Andrew Lunn
2013-01-23 15:30                                 ` Soeren Moch
2013-01-23 16:25                                   ` Andrew Lunn
2013-01-23 17:07                                     ` Soeren Moch
2013-01-23 17:20                                       ` Soeren Moch
2013-01-23 18:10                                         ` Andrew Lunn
2013-01-28 20:59                                           ` Soeren Moch
2013-01-29  0:13                                             ` Jason Cooper
2013-01-29 11:02                                             ` Andrew Lunn
2013-01-29 11:50                                               ` Soeren Moch
2013-01-19 20:05                               ` Arnd Bergmann
2013-01-21 15:01                                 ` Soeren Moch [this message]
2013-01-21 18:55                                   ` Arnd Bergmann
2013-01-21 21:01                                     ` Greg KH
2013-01-22 18:13                                       ` Arnd Bergmann
2013-01-23 14:37                                         ` Soeren Moch
2013-01-19 16:24                             ` Andrew Lunn
2013-01-15 20:05           ` Sebastian Hesselbarth
2013-01-15 20:19             ` Jason Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50FD5844.1010201@web.de \
    --to=smoch@web.de \
    --cc=akpm@linux-foundation.org \
    --cc=andrew@lunn.ch \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=jason@lakedaemon.net \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.cz \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).