Hi Hiroshi,

On Tuesday, August 21, 2012 2:15 PM Hiroshi Doyu wrote:

Hiroshi Doyu [off-list ref] wrote @ Tue, 21 Aug 2012 13:22:35 +0200:

quoted

Hi,

On Mon, 30 Jul 2012 10:28:19 +0200
Marek Szyprowski [off-list ref] wrote:

quoted

This patch changes dma-mapping subsystem to use generic vmalloc areas
for all consistent dma allocations. This increases the total size limit
of the consistent allocations and removes platform hacks and a lot of
duplicated code.

Atomic allocations are served from special pool preallocated on boot,
because vmalloc areas cannot be reliably created in atomic context.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/kernel-parameters.txt |    2 +-
 arch/arm/include/asm/dma-mapping.h  |    2 +-
 arch/arm/mm/dma-mapping.c           |  486 ++++++++++++-----------------------
 arch/arm/mm/mm.h                    |    3 +
 include/linux/vmalloc.h             |    1 +
 mm/vmalloc.c                        |   10 +-
 6 files changed, 181 insertions(+), 323 deletions(-)

...

quoted

@@ -1117,61 +984,32 @@ static int __iommu_free_buffer(struct device *dev, struct page

**pages, size_t s

quoted

quoted

  * Create a CPU mapping for a specified pages
  */
 static void *
-__iommu_alloc_remap(struct page **pages, size_t size, gfp_t gfp, pgprot_t prot)
+__iommu_alloc_remap(struct page **pages, size_t size, gfp_t gfp, pgprot_t prot,
+                   const void *caller)
 {
-       struct arm_vmregion *c;
-       size_t align;
-       size_t count = size >> PAGE_SHIFT;
-       int bit;
+       unsigned int i, nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+       struct vm_struct *area;
+       unsigned long p;

-       if (!consistent_pte[0]) {
-               pr_err("%s: not initialised\n", __func__);
-               dump_stack();
+       area = get_vm_area_caller(size, VM_ARM_DMA_CONSISTENT | VM_USERMAP,
+                                 caller);
+       if (!area)

This patch replaced the custom "consistent_pte" with
get_vm_area_caller()", which breaks the compatibility with the
existing driver. 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()"(*2). Regardless of the badness of allocation
from interrupt handler in the driver, I have the following question.

The following "__get_vm_area_node()" can take gfp_mask, it means that
this function is expected to be called from atomic context, but why
it's _NOT_ allowed _ONLY_ from interrupt context?

According to the following definitions, "in_interrupt()" is in "in_atomic()".

#define in_interrupt()	(preempt_count() & (HARDIRQ_MASK | SOFTIRQ_MASK | NMI_MASK))
#define in_atomic()	((preempt_count() & ~PREEMPT_ACTIVE) != 0)

Does anyone know why BUG_ON(in_interrupt()) is set in __get_vm_area_node(*3)?

For arm_dma_alloc(), it allocates from the pool if GFP_ATOMIC, but for
arm_iommu_alloc_attrs() doesn't have pre-allocate pool at all, and it
always call "get_vm_area_caller()". That's why it hits BUG(). But
still I don't understand why it's not BUG_ON(in_atomic) as Russell
already pointed out(*1).

*1: http://article.gmane.org/gmane.linux.kernel.mm/76708

Ok, now I see the problem. I will try to find out a solution for your issue.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center