block2mtd oops in erase function.

Hi, I have a small fix for a crash that happened when I was using jffs2 in combination with block2mtd.c

In the erase function when checking the block to see if it's already erased, the limit is to be set to the page_address(page) + PAGE_SIZE, but because the variable has the type (ulong *), it gets set to PAGE_SIZE*sizeof(ulong), which makes the kernel oops when the page is very close to the end of RAM.

@@ -111,7 +111,7 @@
 		if (IS_ERR(page))
 			return PTR_ERR(page);
 
-		max = (u_long*)page_address(page) + PAGE_SIZE;
+		max = (u_long*) ((u8 *) page_address(page) + PAGE_SIZE);
 		for (p=(u_long*)page_address(page); p<max; p++)
 			if (*p != -1UL) {
 				lock_page(page);

Good catch! What a twisted little piece of code you've found.

He who knows others is wise.
He who knows himself is enlightened.
-- Lao Tsu

On December 21st 2004 by none other than me. How embarrassing!
http://lists.infradead.org/pipermail/linux-mtd/2004-December/011236.html

Can you test this patch?

index f9f2ce7..6a9fb80 100644
@@ -111,8 +111,8 @@ static int _block2mtd_erase(struct block
 		if (IS_ERR(page))
 			return PTR_ERR(page);
 
-		max = (u_long*)page_address(page) + PAGE_SIZE;
-		for (p=(u_long*)page_address(page); p<max; p++)
+		max = page_address(page) + PAGE_SIZE;
+		for (p=page_address(page); p<max; p++)
 			if (*p != -1UL) {
 				lock_page(page);
 				memset(page_address(page), 0xff, PAGE_SIZE);

Jörn

"[One] doesn't need to know [...] how to cause a headache in order
to take an aspirin."
-- Scott Culp, Manager of the Microsoft Security Response Center, 2001

Cool! Would you mind also trying two more patches? One of them just removes a compile warning if CONFIG_MODULE=n, the other rips out the readahead code, which should give a nice performance boost on USB sticks, MMC/SD, etc. Mounting JFFS2 in particular should be faster.

Jörn

It does not matter how slowly you go, so long as you do not stop.
-- Confucius

I'm using jffs2 on block2mtd. I'd be happy to test any patches you have.

The readahead-disable one in particular is interesting - i ended up patching the source to get rid of the warning about readahead past end-of-disk. And mounting jffs2-on-block2mtd-on-ide-flash is quite slow.

Jason

---
 drivers/mtd/devices/block2mtd.c |    1 -
 1 file changed, 1 deletion(-)

Index: linux-2.6.20-rc5-uml/drivers/mtd/devices/block2mtd.c
===================================================================
@@ -63,7 +63,6 @@
 	for (i = 0; i < PAGE_READAHEAD; i++) {
 		pagei = index + i;
 		if (pagei > end_index) {
-			INFO("Overrun end of disk in cache readahead\n");
 			break;
 		}
 		page = radix_tree_lookup(&mapping->page_tree, pagei);

Is it ok if I just pass this URL?
http://kernel.org/git/?p=linux/kernel/git/joern/misc.git

The top three patches are test-worthy.

If it works for you and improves mount time, that would be good results.

Jörn

I've never met a human being who would want to read 17,000 pages of
documentation, and if there was, I'd kill him to get him out of the
gene pool.
-- Joseph Costello

Great!

Ouch! While this is a significant improvement, 54s is still quite bad. The 512MiB partition on the OLPC is mounting in ~3.4s with JFFS2 and ~60ms with LogFS. Imo 3.4s is already bad, and 54s is way beyond.

Now where is time lost? One candidate is erase block summary, enabling it usually gives roughly a 6x performance improvement. That would get you down to ~9s.

Another next thing I suspect is the erase size. The default is 4KiB, which is rather small. Can you try setting it to 64KiB or maybe even 128KiB?

Instead of passing "block2mtd=/dev/foo", you can pass "block2mtd=/dev/foo,64KiB" if my memory doesn't fail me.

Changing the erase size to a multiple of the previous erase size should be fine with JFFS2 (a smaller erase size could lead to data loss), but you likely won't get better performance unless you recreate the filesystem. Maybe something like

$ mount /dev/mtdX /mnt -t jffs2
$ (cd /mnt && tar cvpf /tmp/foo.tgz .)
$ umount /mnt
$ flash_eraseall /dev/mtdX
$ mount /dev/mtdX /mnt -t jffs2
$ (cd /mnt && tar vpf /tmp/foo.tgz)
$ umount /dev/mtdX

Unless you are happy with the current mount time, of course. Noone is forcing you to change anything.

Jörn

Do not stop an army on its way home.
-- Sun Tzu

How do I do that?

Those timings are already using a 128k erase size.

A raw read of the underlying block device takes 54s, so jffs2 isn't adding any significant overhead anymore. It was when readahead was in use.

The only way to increase performance at this point is to make it so that jffs2 doesn't need to read the entire device at mount time. Is this what erase block summaries do?

Jason Lunz | Senior Developer | Reflex Security, Inc.
[email protected]

That is a bit surprising. JFFS2 has some optimizations to skip empty blocks after some bytes. This is where readahead really hurt - JFFS2 would skip and readahead would read the skipped data anyway. Now I can see that JFFS2 has some overhead over plain reading the device, but it is unusual that overhead and optimizations cancel each other out so perfectly.

Yes. The idea is to only read the summary at the end of an erase block. If the summary does not exist, it will fall back to linear scanning of the block, afaik.

If you enable CONFIG_JFFS2_SUMMARY, it should work for you. Of course, your filesystem needs the summary, so you either have to do the backup&restore procedure or apply sumtool (is that its name?) to your images.

Jörn

Joern's library part 3:
http://inst.eecs.berkeley.edu/~cs152/fa05/handouts/clark-test.pdf

I thought I'd enabled CONFIG_JFFS2_SUMMARY, but it turns out not. I've enabled it now, but I still don't seem to see any speedup.

I'm unclear on how to make the filesystem use it. I create the jffs2 filesystem by simply doing:

flash_eraseall -j /dev/mtd0
mount -t jffs2 mtd0 /state

Will this create a summary automatically, or is there an extra step I need to take?

...ah, it seems removing the -j does the trick. If I don't use it, mount goes from >50s to <4s.

thanks!

Jason

That's sounds better. Thank you for testing the patches.

Jörn

Time? What's that? Time is only worth what you do with it.
-- Theo de Raadt