jffs2 summary allocation

Hi,

I apply this patch to fix this oops.

Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c0004000
[00000000] *pgd=00000000
stopped custom tracer.
Internal error: Oops: 817 [#1] PREEMPT
Modules linked in:
CPU: 0    Not tainted  (2.6.24-rc5-rt1 #37)
PC is at dma_cache_maint+0x40/0x80
LR is at atmel_spi_transfer+0x94/0x178
pc : [<c002488c>]    lr : [<c013eedc>]    psr: 20000013

sp : c044db84 ip : c044db94 fp : c044db90
r10: ffffffff r9 : 00000000 r8 : c04e4c00
r7 : c03ee310 r6 : c044dcfc r5 : c109d3bc r4 : c044dcd8 r3 : 00000000 r2 : 00000001 r1 : c109d7dc r0 : c109d3bc
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 0005317f Table: 20588000 DAC: 00000017 Process jffs2_gcd_mtd1 (pid: 313, stack limit = 0xc044c258)

Stack: (0xc044db84 to 0xc044e000)

db80: c044dbb8 c044db94 c013eedc c002485c c04e4c00 c044dbbc c044dcfc
dba0: c044dc20 00000000 c044dcfc c044dca4 c044dbbc c013e124 c013ee58 00000000
dbc0: 00000001 dead4ead ffffffff ffffffff c02e7844 00000000 c01933d4 c044dc40
dbe0: c044dc40 c044dc48 c044dc48 c044dc24 00000000 005593e0 c018f8b4 c044607c
dc00: 00000420 c044dc88 c044dc78 c01defa8 00000000 c018f8b4 c044dc7c c044dc7c
dc20: 00000000 00000001 dead4ead ffffffff ffffffff c02e7844 00000000 c01933d4
dc40: c044dc40 c044dc40 c044dc48 c044dc48 c044dc24 00000000 005593e0 c018f8b4
dc60: c044607c 00000420 c044dc88 c044dc78 c01defa8 00000000 c018f8b4 c044dc7c
dc80: c044dc7c 00a5f800 00000420 c0446000 00000420 000014bf c044dd50 c044dca8
dca0: c013dcb0 c013e0c0 c0446024 c04e4c00 c109d3bc c0446000 00000000 00000004
dcc0: 20446000 ffffffff 00000000 00000000 c044dcf4 c044dcfc c109d3bc 00000000
dce0: 00000420 ffffffff ffffffff 00000000 00000000 c044dcfc c044dcd0 c044dcd0
dd00: c044dcf4 c04e4c00 00000000 c013e148 c044dc20 00000000 00000000 00000000
dd20: 00000000 00000000 00000000 00427fe0 00000000 c109d3bc 00427fe0 000003dc
dd40: 00000000 c044dd74 c044dd54 c013a00c c013db10 c044dd9c c109d3bc 00000420
dd60: 00000420 c04d9000 c044ddc8 c044dd78 c00e0724 c0139f74 c044dd9c c109d3bc
dd80: 00000002 c044de08 c04d93a0 00427c24 00000001 c044de10 00427c04 00000000
dda0: c044ed70 00427c04 c109d048 c04d9000 00000000 0000078c c056689c c044de44
ddc0: c044ddd0 c00e1f40 c00e04c0 00427c04 00000000 c044de18 00000000 c04d91cc
dde0: 000007fc 000007dc 20061985 000007fc 37fa2120 00000004 00000000 00000364
de00: 36d8bf4d b504e572 c044dde8 00000020 c109d000 000007dc 00000000 c044ed70
de20: c056689c 00000938 c04d9000 000008c8 000000c4 c04d91cc c044de84 c044de48
de40: c00d5ca0 c00e1c1c c044de68 c044de58 c04d91b4 000000c4 c044dec4 c044ed70
de60: c04d9000 000000c4 c04d91cc 000008c8 c044ed70 c044dec4 c044deac c044de88
de80: c00d6014 c00d5c1c c044ed70 000008c8 c0568950 c04d9000 c04d9000 c037bde4
dea0: c044def4 c044deb0 c00da938 c00d5fe4 c04d92e8 c044c000 00000001 c044dec8
dec0: c014e3c0 000000c4 c04d92e8 c044ed70 c04e1ecc c0568950 c037bde4 c04d9000
dee0: c04d90dc c04d92e8 c044df48 c044def8 c00dc32c c00da8ec c044df04 c04d9194
df00: c04d9174 c04d91c4 000015e8 c044c000 c044df30 c044df20 c014ecfc c014e37c
df20: 00000001 c039c3e0 c044c000 c04d9000 00000008 00000002 c044df4c c044dff4
df40: c044df4c c00dd730 c00dbdac 00000001 00000000 00000080 00000000 00000000
df60: 00000000 00000000 00000000 c044df98 c044df7c c0060ea0 c005bd74 c0324020
df80: 00000000 c0324020 00000000 c044dfac c044df9c c00326f4 c0060f78 00000000
dfa0: 00000000 c044dfb0 c001ff44 c00326c4 00000000 c04d9000 c00dd630 c0039818
dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
dfe0: 00000000 00000000 00000000 c044dff8 c0039818 c00dd640 00000000 00000000

Backtrace:
[<c002484c>] (dma_cache_maint+0x0/0x80) from [<c013eedc>] (atmel_spi_transfer+0x94/0x178)
[<c013ee48>] (atmel_spi_transfer+0x0/0x178) from [<c013e124>] (spi_sync+0x74/0x98)
[<c013e0b0>] (spi_sync+0x0/0x98) from [<c013dcb0>] (dataflash_write+0x1b0/0x270)
 r8:000014bf r7:00000420 r6:c0446000 r5:00000420 r4:00a5f800
[<c013db00>] (dataflash_write+0x0/0x270) from [<c013a00c>] (part_write+0xa8/0xb0)
[<c0139f64>] (part_write+0x0/0xb0) from [<c00e0724>] (jffs2_flash_writev+0x278/0x434)
 r6:c04d9000 r5:00000420 r4:00000420
[<c00e04b0>] (jffs2_flash_writev+0x4/0x434) from [<c00e1f40>] (jffs2_sum_write_sumnode+0x334/0x420)
[<c00e1c0c>] (jffs2_sum_write_sumnode+0x0/0x420) from [<c00d5ca0>] (jffs2_do_reserve_space+0x94/0x3c8)
[<c00d5c0c>] (jffs2_do_reserve_space+0x0/0x3c8) from [<c00d6014>] (jffs2_reserve_space_gc+0x40/0x78)
[<c00d5fd4>] (jffs2_reserve_space_gc+0x0/0x78) from [<c00da938>] (jffs2_garbage_collect_pristine+0x5c/0x3a8)
[<c00da8dc>] (jffs2_garbage_collect_pristine+0x0/0x3a8) from [<c00dc32c>] (jffs2_garbage_collect_pass+0x590/0x714)
[<c00dbd9c>] (jffs2_garbage_collect_pass+0x0/0x714) from [<c00dd730>] (jffs2_garbage_collect_thread+0x100/0x18c)
[<c00dd630>] (jffs2_garbage_collect_thread+0x0/0x18c) from [<c0039818>] (do_exit+0x0/0x73c)
Code: 9a000001 e15c0003 3a000001 e3a03000 (e5833000)

Inviato da Yahoo! Mail.
La casella di posta intelligente.
http://it.docs.yahoo.com/mail/overview/index.html

Change the summary buffer allocation for resolving an oops during dma transfer using spi bus.

---
 fs/jffs2/summary.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

index 629af01..5962b3b 100644
@@ -17,7 +17,6 @@
 #include <linux/pagemap.h>
 #include <linux/crc32.h>
 #include <linux/compiler.h>
-#include <linux/vmalloc.h>
 #include "nodelist.h"
 #include "debug.h"
 

@@ -30,7 +29,7 @@ int jffs2_sum_init(struct jffs2_sb_info *c)
 		return -ENOMEM;
 	}
 
-	c->summary->sum_buf = vmalloc(c->sector_size);
+	c->summary->sum_buf = kmalloc(c->sector_size, GFP_KERNEL);
 
 	if (!c->summary->sum_buf) {
 		JFFS2_WARNING("Can't allocate buffer for writing out summary information!\n");

@@ -49,7 +48,7 @@ void jffs2_sum_exit(struct jffs2_sb_info *c)
 
 	jffs2_sum_disable_collecting(c->summary);
 
-	vfree(c->summary->sum_buf);
+	kfree(c->summary->sum_buf);
 	c->summary->sum_buf = NULL;
 
 	kfree(c->summary);

1.5.2.1.174.gcd03-dirty

@@ -17,7 +17,6 @@
 #include <linux/pagemap.h>
 #include <linux/crc32.h>
 #include <linux/compiler.h>
-#include <linux/vmalloc.h>
 #include "nodelist.h"
 #include "debug.h"
 

@@ -30,7 +29,7 @@ int jffs2_sum_init(struct jffs2_sb_info 
 		return -ENOMEM;
 	}
 
-	c->summary->sum_buf = vmalloc(c->sector_size);
+	c->summary->sum_buf = kmalloc(c->sector_size, GFP_KERNEL);
 
 	if (!c->summary->sum_buf) {
 		JFFS2_WARNING("Can't allocate buffer for writing out summary information!\n");

@@ -49,7 +48,7 @@ void jffs2_sum_exit(struct jffs2_sb_info
 
 	jffs2_sum_disable_collecting(c->summary);
 
-	vfree(c->summary->sum_buf);
+	kfree(c->summary->sum_buf);
 	c->summary->sum_buf = NULL;
 
 	kfree(c->summary);

_

All this does is switch sum_buf from vmalloced-memory over to kmalloced-memory.

I'm assuming from the trace that the arm code tried to put that memory under DMA (or at least, passed it into part of the DMA management code to get the various caches sorted out) and that the arm DMA support code doesn't like being given vmalloced memory.

So the question is: who is wrong here? Is jffs wrong to use vmalloced memory in this application, or is arm wrong to not handle it?

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

If ARM is wrong, we're going to have to walk page tables and all that mess so that we can perform L2 cache maintainence for DMA... which becomes quite expensive and I believe starts to make DMA pointless on ARM CPUs.

The DMA docs say this of dma_map_coherent():

Notes: Not all memory regions in a machine can be mapped by this API. Further, regions that appear to be physically contiguous in kernel virtual space may not be contiguous as physical memory. Since this API does not provide any scatter/gather capability, it will fail if the user tries to map a non-physically contiguous piece of memory. For this reason, it is recommended that memory mapped by this API be obtained only from sources which guarantee it to be physically contiguous (like kmalloc).

vmalloc memory is not physically contiguous, so the attempt to create the mapping fails.

-------------------------------------------------------------------

List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

Actually, Documentation/DMA-Mapping.txt has a section right up front called "What memory is DMA'able?" ... which despite its ungrammatical title, says clearly:

... This means specifically that you may _not_ use the memory/addresses returned from vmalloc() for DMA. ...

So I'm rather surprised to see *ANY* kernel code trying to do that. That rule has been in effect for many, many years now.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

I don't think it was intentional. You're going through several layers here:

JFFS2 -> mtd parts -> mtd dataflash -> atmel_spi.

Typically MTD drivers aren't doing DMAs to flash and JFFS2 has no idea which particular chip driver is being used because it's abstracted by MTD.

josh

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

That's true ... although I can imagine using DMA to avoid dcache trashing if its setup cost is low enough, with either NAND or NOR chips.

Still: in this context vmalloc() is wrong.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Agreed. One issue is that the summary code allocates a buffer that equals the eraseblock size of the underlying MTD device. For larger NAND chips, that may be up to 256KiB. I believe this is within the allowable kmalloc size for most architectures these days, but the summary code is 3 years old and was likely expecting a smaller limit. And there is always the question on whether finding that much contiguous memory will be an issue.

I don't see much harm with the actual patch itself, assuming larger kmallocs work as I think they should. It does make me wonder if we have other cases of vmalloc'd buffers being passed to lower drivers using DMA though.

josh

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

In MLC chips it goes up to 512KiB. It means it can't allocate the eraseblock size memory with kmalloc().
In ARM environment I can't see the 256KiB or more memory allocation with kmalloc().
So I now changed the kmalloc eraseblock to vmalloc at both jffs2 and mtd-utils.

Thank you,
Kyungmin Park

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Yes. This is why I'm reluctant to whizz this patch into 2.6.25. It'll break more than it fixes.

Does this eraseblock really really really need to be a single virtually-contiguous hunk of kernel memory? Or was that just easy to do at the time?

This problem comes up pretty often. Rather than open-coding it yet again it'd be nice to have a little bit of library code which manages an array of pages and which has accessors for common operations like read/write-u8/u16/u32/u64, memset, memcpy, etc.

Then again, given that this memory is often fed into IO subsystems, perhaps we should do this by adding more accessors and helpers to scatterlists/sg_table. Unfortunately they're not presently well set up for random access.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Which problem -- kmalloc(BIG)? Or dma(dma-unsafe-mem)? Or something else?

The specific issue here might best be described as JFFS2 making a bad assumption: that MTD drivers never use DMA. The I2C stack does that in some cases (for example, I/O buffers in i2c_smbus_xfer_emulated are on-stack), but I'd not call that an especially common assumption.

If array-of-pages is to be more widely adopted, that'd make sense. The MTD framework is a bit odd in that respect ... it has block devices but doesn't use the scatterlist primitives.

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Code needing a large amount of memory, trying to kmalloc it, then getting into trouble, them using vmalloc, then getting into more trouble.

This is kernel - order-0 allocations will always be the most robust.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

I hope you meant: vmalloc changed to kmalloc?

And mtd-utils is userspace, not kernel, so it can't care...

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone