Check the given range("start", "size") is included in "atomic_pool" or not.
Signed-off-by: Hiroshi Doyu <redacted>
---
arch/arm/mm/dma-mapping.c | 25 +++++++++++++++++++------
1 files changed, 19 insertions(+), 6 deletions(-)
Use kzalloc() with GFP_ATOMIC instead of vzalloc(). At freeing,
__in_atomic_pool() checks if it comes from atomic_pool or not.
Signed-off-by: Hiroshi Doyu <redacted>
---
arch/arm/mm/dma-mapping.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
Makes use of the same atomic pool from DMA, and skips kernel page
mapping which can involve sleep'able operations at allocating a kernel
page table.
Signed-off-by: Hiroshi Doyu <redacted>
---
arch/arm/mm/dma-mapping.c | 30 +++++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 5 deletions(-)
From: Minchan Kim <minchan@kernel.org> Date: 2012-08-23 06:43:33
On Thu, Aug 23, 2012 at 09:10:25AM +0300, Hiroshi Doyu wrote:
Hi,
The commit e9da6e9 "ARM: dma-mapping: remove custom consistent dma
region" breaks the compatibility with existing drivers. This causes
the following kernel oops(*1). That driver has called dma_pool_alloc()
to allocate memory from the interrupt context, and it hits
BUG_ON(in_interrpt()) in "get_vm_area_caller()". This patch seris
fixes this problem with making use of the pre-allocate atomic memory
pool which DMA is using in the same way as DMA does now.
Any comment would be really appreciated.
v2:
Don't modify attrs(DMA_ATTR_NO_KERNEL_MAPPING) for atomic allocation. (Marek)
Skip vzalloc (KyongHo, Minchan)
Huh? I would like to correct exactly. I didn't say that kzalloc unify is okay.
As KyongHo said, there are other usecases for allocating big buffer but
I can't agree his opinion that it's system memory shortage if the allocation
fails. Because there are lots of freeable pages(cached page + anon pages
with swap , shrinkable slab and so on). But as I said early, VM can't do
anything except relying on kswapd in atomic context while there are a ton
of freeable pages in system and we can't gaurantee kswapd will do something
for us before seeing the page allocation failure in process context.
Especially, UP is more problem. So, it never indicate system needs more memory.
Atomic high order allocation is very fragile in VM POV
so caller should have fallback mechanism. Otherwise, don't do that.
--
Kind regards,
Minchan Kim
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2012-08-23 07:33:28
Hi Hiroshi,
On Thursday, August 23, 2012 8:10 AM Hiroshi Doyu wrote:
quoted hunk
Makes use of the same atomic pool from DMA, and skips kernel page
mapping which can involve sleep'able operations at allocating a kernel
page table.
Signed-off-by: Hiroshi Doyu <redacted>
---
arch/arm/mm/dma-mapping.c | 30 +++++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 5 deletions(-)
size_t s
int count = size >> PAGE_SHIFT;
int array_size = count * sizeof(struct page *);
int i;
+
+ if (__free_from_pool(page_address(pages[0]), size))
+ goto out;
+
for (i = 0; i < count; i++)
if (pages[i])
__free_pages(pages[i], 0);
+
+out:
if ((array_size <= PAGE_SIZE) ||
__in_atomic_pool(page_address(pages[0]), size))
kfree(pages);
@@ -1221,6 +1237,9 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, if (*handle == DMA_ERROR_CODE) goto err_buffer;+ if (gfp & GFP_ATOMIC)+ return page_address(pages[0]);+ if (dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs)) return pages;
I've read the whole code and it looks that it starts to become a little spaghetti - there are too
many places altered by those atomic changes and it is hard to follow the logic at the first sight.
IMHO it will be better to add a following function:
static void *__iommu_alloc_atomic(struct device *dev, size_t size,
dma_addr_t *handle)
{
struct page *page, **pages;
int count = size >> PAGE_SHIFT;
int array_size = count * sizeof(struct page *);
void *addr;
int i;
pages = kzalloc(array_size, gfp);
if (!pages)
return NULL;
addr = __alloc_from_pool(size, &page);
if (!addr)
goto err_pool;
for (i = 0; i < count; i++)
pages[i] = page + i;
*handle = __iommu_create_mapping(dev, pages, size);
if (*handle == DMA_ERROR_CODE)
goto err_mapping;
return addr;
err_mapping:
__free_from_pool(addr, size);
err_pool:
kfree(pages);
return NULL;
}
and then call it@the beginning of the arm_iommu_alloc_attrs():
if (gfp & GFP_ATOMIC)
return __iommu_alloc_atomic(dev, size, handle);
You should also add support for the allocation from atomic_pool to __iommu_get_pages() function
to get dma_mmap() and dma_get_sgtable() working correctly.
Are you sure this one works? __iommu_get_pages() won't find pages array, because atomic
allocations don't get their separate vm_struct area, so this check will be never reached. Maybe
a __iommu_free_atomic() function would also make it safer and easier to understand.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center