Patchwork ubifs_decompress: cannot decompress ...

login
register
mail settings
Submitter Artem Bityutskiy
Date June 9, 2011, 12:10 p.m.
Message ID <1307621434.7374.78.camel@localhost>
Download mbox | patch
Permalink /patch/99741/
State New
Headers show

Comments

Artem Bityutskiy - June 9, 2011, 12:10 p.m.
On Wed, 2011-06-08 at 13:50 -0400, Matthew L. Creech wrote:
> On Wed, Jun 8, 2011 at 10:11 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> >
> > Yes, it does look like this LEB might be garbage-collected. But it does
> > not have to be.
> >
> > Anyway, what I can suggest you is to do several things.
> >
> > 1. If you have many occasions of such error, try to gather some
> >   information about how the device was used, and if it was uncleanly
> >   power-cut. Remember, I often saw that embedded devices have incorrect
> >   reboot. Whe users reboot it "normally" - it does not try to unmount
> >   the FS-es cleanly and just jumps to som HW reset function.
> >
> >   You can verify this by rebooting normally and checking if UBIFS says
> >   "recovery needed" or not. If it does - the reboot was not normal.
> >
> 
> Yes, it currently reboots uncleanly (though it does do a "sync"
> first).  I noticed this a while back, and the next release firmware
> will have it fixed.  However, it doesn't make a huge difference to us,
> because these devices are probably more likely to experience power
> loss than a software reboot, in the field at least.
> 
> > 2. This error may be due to memory corruptions in some driver (e.g.,
> >   wireless or video), due to issues in the mtd driver, etc. Try to
> >   stress your system with slub/slab full checks enabled, and other
> >   debugging features which you can find in the "hacking" section of
> >   make menuconfig.
> >
> 
> Will do.
> 
> > 3. If my theory is true, then what may help is adding a check it
> >   ubifs recovery function. The recovery ends with an ubifs_leb_change()
> >   call. You need to check the last node there - is it full and correct?
> >   If not, you should print a loud warning and information like leb dump
> >   _before_ the change, and dump of the buffer which we are going to
> >   write with ubifs_leb_change().
> >
> >   You'd probably need to deploy this check to the field if this issue
> >   is not easy to reproduce. If you have then this info you may fix the
> >   bug.
> >
> 
> Great, I'll add this check and see if we get any hits.  Even if it
> takes a while to hit it in the field, this would at least give us a
> way to make some progress in finding the issue.

With my latest code-base, I am able to inject a hack into
ubifs_leb_change() - but this function does not exist in your code-base.
Anyway, I'm currently running power cut emulation testing with the
following hack:


From df163f4dd8a1604fe3085c4d11281c530837bc53 Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Date: Thu, 9 Jun 2011 15:08:59 +0300
Subject: [PATCH] UBIFS: temporary: hack to check recovery

We suspect that recovery cuts nodes sometimes. This is the hack which should
catch such things. We hack ubifs_change_leb and scan the leb right after
changing it - if we wrote corrupted data there, scan should fail.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/io.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)
Matthew L. Creech - June 20, 2011, 3:35 p.m.
On Thu, Jun 9, 2011 at 8:10 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> With my latest code-base, I am able to inject a hack into
> ubifs_leb_change() - but this function does not exist in your code-base.
> Anyway, I'm currently running power cut emulation testing with the
> following hack:
>

Hi Artem, just wanted to post an update to this:

I hacked this change into my kernel last week, and enabled the
in-kernel debugging options you suggested, then set up a power-cut
test:

- the device in question is powered by a UPS with controllable outlets
- its firmware boots and immediately starts writing large amounts of
data to flash (so we'll have a dirty FS)
- a separate script controlling the UPS waits for the device to boot,
then sleeps some random time between 0 and 10 minutes
- it then cuts power to the outlet and later restores it

The test has been running for the better part of a week with no issues
- it has now power-cycled the device over 1500 times, but I've gotten
no errors, and the filesystem is functioning fine.  FYI this is way
more stress than was placed on the "bad" units we've gotten back from
the field - most of those rarely lose power, and when they do, they're
less likely to be in the middle of a write operation.

I'll continue testing, but based on this it seems like UBIFS handles
power cuts very well!

Patch

diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c
index 9228950..9f7dbbf 100644
--- a/fs/ubifs/io.c
+++ b/fs/ubifs/io.c
@@ -153,6 +153,30 @@  int ubifs_leb_change(struct ubifs_info *c, int lnum, const void *buf, int len,
 		ubifs_ro_mode(c, err);
 		dbg_dump_stack();
 	}
+
+	/* Temporary hack to catch incorrect recovery, if we have such */
+	if (!err && (lnum < c->lpt_first || lnum > c->lpt_last)) {
+		void *buf = vmalloc(c->leb_size);
+		struct ubifs_scan_leb *sleb;
+
+		if (!buf)
+			return 0;
+
+		sleb = ubifs_scan(c, lnum, 0, buf, 0);
+		if (!IS_ERR(sleb)) {
+			/* Scan succeeded */
+			vfree(buf);
+			return 0;
+		}
+
+		ubifs_err("scanning after LEB %d change failed, error %d!", lnum, err);
+		print_hex_dump(KERN_ERR, "", DUMP_PREFIX_OFFSET, 32, 1,
+			       buf, c->leb_size, 1);
+		dump_stack();
+		vfree(buf);
+		return -EINVAL;
+	}
+
 	return err;
 }