[UBI UBIFS] replace vmalloc with kmalloc

[UBI UBIFS] replace vmalloc with kmalloc

[JiSheng Zhang]

If nandflash driver use DMA to support UBI and UBIFS i/o, there will
be panic like
the following. This is because UBI and UBIFS use vmalloced buffer
somewhere while
nandflash driver put the buffer under DMA and DMA support doesn't like vmalloced
memory.
[  412.369280] UBIFS: default file-system created
[  412.374879] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[  412.383177] UBIFS: background thread "ubifs_bgt0_0" started, PID 887
[  412.389656] pgd = c0004000
[  412.392419] [00000000] *pgd=00000000
[  412.396117] Internal error: Oops: 817 [#1] PREEMPT

Yes, I know kmalloc may fail when memory get fragmented, but I grep the UBI and
UBIFS's source, there are six .c files and seven .c files using
vmalloc, the function
which call vmalloc are all called during the ubi being attached and
the volume being
mounted. In embedded system, this mainly happen during kernel boot, it
should succeed.

After replacing vmalloc with kmalloc, ubi and ubifs runs fine on my
board. I can alos upload
a patch.

Comments are welcomed!

Jisheng

Re: [UBI UBIFS] replace vmalloc with kmalloc

[Matthieu CASTET]

JiSheng Zhang a écrit :
> If nandflash driver use DMA to support UBI and UBIFS i/o, there will
> be panic like
> the following. This is because UBI and UBIFS use vmalloced buffer
> somewhere while
> nandflash driver put the buffer under DMA and DMA support doesn't like vmalloced
> memory.
> [  412.369280] UBIFS: default file-system created
> [  412.374879] Unable to handle kernel NULL pointer dereference at
> virtual address 00000000
> [  412.383177] UBIFS: background thread "ubifs_bgt0_0" started, PID 887
> [  412.389656] pgd = c0004000
> [  412.392419] [00000000] *pgd=00000000
> [  412.396117] Internal error: Oops: 817 [#1] PREEMPT
> 
> Yes, I know kmalloc may fail when memory get fragmented, but I grep the UBI and
> UBIFS's source, there are six .c files and seven .c files using
> vmalloc, the function
> which call vmalloc are all called during the ubi being attached and
> the volume being
> mounted. In embedded system, this mainly happen during kernel boot, it
> should succeed.
> 
> After replacing vmalloc with kmalloc, ubi and ubifs runs fine on my
> board. I can alos upload
> a patch.
> 
> Comments are welcomed!
> 
mtd is not DMA safe. So patching UBI and UBIFS is not enought.


Matthieu

Re: [UBI UBIFS] replace vmalloc with kmalloc

[Adrian Hunter]

JiSheng Zhang wrote:
> If nandflash driver use DMA to support UBI and UBIFS i/o, there will
> be panic like
> the following. This is because UBI and UBIFS use vmalloced buffer
> somewhere while
> nandflash driver put the buffer under DMA and DMA support doesn't like vmalloced
> memory.
> [  412.369280] UBIFS: default file-system created
> [  412.374879] Unable to handle kernel NULL pointer dereference at
> virtual address 00000000
> [  412.383177] UBIFS: background thread "ubifs_bgt0_0" started, PID 887
> [  412.389656] pgd = c0004000
> [  412.392419] [00000000] *pgd=00000000
> [  412.396117] Internal error: Oops: 817 [#1] PREEMPT
> 
> Yes, I know kmalloc may fail when memory get fragmented, but I grep the UBI and
> UBIFS's source, there are six .c files and seven .c files using
> vmalloc, the function
> which call vmalloc are all called during the ubi being attached and
> the volume being
> mounted. In embedded system, this mainly happen during kernel boot, it
> should succeed.
> 
> After replacing vmalloc with kmalloc, ubi and ubifs runs fine on my
> board. I can alos upload
> a patch.
> 
> Comments are welcomed!

vmalloc allows large (> 128KiB) buffers, but kmalloc doesn't.
So we presently have no choice but to use vmalloc.

I do not know what hardware you have or exactly what driver you
are using, but we have UBIFS on OneNAND using DMA.
See drivers/mtd/onenand/omap2.c

Re: [UBI UBIFS] replace vmalloc with kmalloc

[Russell King]

On Fri, Aug 07, 2009 at 12:20:03PM +0300, Adrian Hunter wrote:
> vmalloc allows large (> 128KiB) buffers, but kmalloc doesn't.
> So we presently have no choice but to use vmalloc.
>
> I do not know what hardware you have or exactly what driver you
> are using, but we have UBIFS on OneNAND using DMA.
> See drivers/mtd/onenand/omap2.c

Hmm.  Looking at that code, it's unsafe:

        if (buf >= high_memory) {
                struct page *p1;

                if (((size_t)buf & PAGE_MASK) !=
                    ((size_t)(buf + count - 1) & PAGE_MASK))
                        goto out_copy;
                p1 = vmalloc_to_page(buf);
                if (!p1)
                        goto out_copy;
                buf = page_address(p1) + ((size_t)buf & ~PAGE_MASK);
        }
...
        dma_dst = dma_map_single(&c->pdev->dev, buf, count, DMA_FROM_DEVICE);

If you consider a VIVT cache, and with vmalloc'd pages you're passing
a *different* virtual address to the DMA functions, it's not going to
touch the cachelines associated with the vmalloc mapping.

The above *may* work for VIPT caches provided both the vmalloc and
kernel direct mappings are of the same colour.  If not, this really
isn't going to be reliable for the same reason.

Basically, what's going in there is *totally* unsafe.  I hope you
don't place important data on this NAND device.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [UBI UBIFS] replace vmalloc with kmalloc

[Josh Boyer]

On Fri, Aug 07, 2009 at 12:20:03PM +0300, Adrian Hunter wrote:
> JiSheng Zhang wrote:
>> If nandflash driver use DMA to support UBI and UBIFS i/o, there will
>> be panic like
>> the following. This is because UBI and UBIFS use vmalloced buffer
>> somewhere while
>> nandflash driver put the buffer under DMA and DMA support doesn't like vmalloced
>> memory.
>> [  412.369280] UBIFS: default file-system created
>> [  412.374879] Unable to handle kernel NULL pointer dereference at
>> virtual address 00000000
>> [  412.383177] UBIFS: background thread "ubifs_bgt0_0" started, PID 887
>> [  412.389656] pgd = c0004000
>> [  412.392419] [00000000] *pgd=00000000
>> [  412.396117] Internal error: Oops: 817 [#1] PREEMPT
>>
>> Yes, I know kmalloc may fail when memory get fragmented, but I grep the UBI and
>> UBIFS's source, there are six .c files and seven .c files using
>> vmalloc, the function
>> which call vmalloc are all called during the ubi being attached and
>> the volume being
>> mounted. In embedded system, this mainly happen during kernel boot, it
>> should succeed.
>>
>> After replacing vmalloc with kmalloc, ubi and ubifs runs fine on my
>> board. I can alos upload
>> a patch.
>>
>> Comments are welcomed!
>
> vmalloc allows large (> 128KiB) buffers, but kmalloc doesn't.
> So we presently have no choice but to use vmalloc.

You could use get_free_pages to get up to 2MiB, no?

josh

Re: [UBI UBIFS] replace vmalloc with kmalloc

[JiSheng Zhang]

Adrian Hunter <adrian.hunter@nokia.com> wrote:
> 
> vmalloc allows large (> 128KiB) buffers, but kmalloc doesn't.
> So we presently have no choice but to use vmalloc.

But vmalloced buffer can't be easily passed to DMA, is there better choice?

> 
> I do not know what hardware you have or exactly what driver you
> are using, but we have UBIFS on OneNAND using DMA.

An arm soc board, the controller driver hasn't been merged upstream.
> See drivers/mtd/onenand/omap2.c
>

One limit of the omap2.c is that the buffer passed to DMA can't be larger than
PAGESIZE?

Jisheng

Re: [UBI UBIFS] replace vmalloc with kmalloc

[Artem Bityutskiy]

On Fri, 2009-08-07 at 23:28 +0800, JiSheng Zhang wrote:
> Adrian Hunter <adrian.hunter@nokia.com> wrote:
> > 
> > vmalloc allows large (> 128KiB) buffers, but kmalloc doesn't.
> > So we presently have no choice but to use vmalloc.
> 
> But vmalloced buffer can't be easily passed to DMA, is there better choice?

We use vmalloc-ed buffers is when we need to do something with whole
eraseblocks, which may be 128KiB, or even 256KiB. So in general, we
cannot just s/kmalloc/vmalloc/. We should invent something trickier.

You should probably think about some generic way to solve problems like
this. May be some MTD-specific library could be created. You could
implement something like the flexible arrays, but with DMA stuff in mind
(e.g., adding a parameter which specifies allocation order, because for
DMA you may want to allocate more than one physically contiguous page).
Or you may even improve the existing flexible arrays to suit your needs.
Take a look here:
http://lwn.net/Articles/345273/

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)