Hiroshi Doyu <hdoyu@xxxxxxxxxx> wrote @ Tue, 21 Aug 2012 13:22:35 +0200:
> Hi,
>
> On Mon, 30 Jul 2012 10:28:19 +0200
> Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
>
> > 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@xxxxxxxxxxx>
> > Reviewed-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > ---
> > 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(-)
> >
> ...
> > @@ -1117,61 +984,32 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages, size_t s
> > * 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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href