Patchwork ubifs : corruption after power cut test

login
register
mail settings
Submitter Matthieu CASTET
Date Aug. 2, 2010, 9:32 a.m.
Message ID <4C5690B1.6030407@parrot.com>
Download mbox | patch
Permalink /patch/60525/
State New
Headers show

Comments

Matthieu CASTET - Aug. 2, 2010, 9:32 a.m.
Matthieu CASTET a écrit :
> Hi,
> 
> Matthieu CASTET a écrit :
>> Artem Bityutskiy a écrit :
>> Ok thanks, I will run it
>>
>> When checking the code, I saw that switch_gc_head can set c->gc_lnum to -1.
>>
>> In ubifs_put_super, we set c->mst_node->gc_lnum to c->gc_lnum and write 
>> master node.
>> Can't ubifs_put_super run while switch_gc_head set gc_lnum to -1 ?
>>
> I manage to reproduce it with the backtrace [1].
> 
Waiting for a proper fix, I force recovery if gc_lnum to -1.


Matthieu
Artem Bityutskiy - Aug. 4, 2010, 4:14 p.m.
On Mon, 2010-08-02 at 11:32 +0200, Matthieu CASTET wrote:
> Matthieu CASTET a écrit :
> > Hi,
> > 
> > Matthieu CASTET a écrit :
> >> Artem Bityutskiy a écrit :
> >> Ok thanks, I will run it
> >>
> >> When checking the code, I saw that switch_gc_head can set c->gc_lnum to -1.
> >>
> >> In ubifs_put_super, we set c->mst_node->gc_lnum to c->gc_lnum and write 
> >> master node.
> >> Can't ubifs_put_super run while switch_gc_head set gc_lnum to -1 ?
> >>
> > I manage to reproduce it with the backtrace [1].
> > 
> Waiting for a proper fix, I force recovery if gc_lnum to 

The workaround looks ok, but I still do not understand how we end up
with writing -1. The only place c->gc_lnum is set to -1 is in GC, but it
is then initialized properly, and only error can cause GC to return with
c->gc_lnum == -1, in which case we switch to R/O mode immediately.

Is your UBIFS identical to what I have in my 2.6.27 back-port tree?

Also, I will have really little time till the beginning of September, so
probably this will wait...

Patch

diff --git a/fs/ubifs/master.c b/fs/ubifs/master.c
index 28beaee..2b668cc 100644
--- a/fs/ubifs/master.c
+++ b/fs/ubifs/master.c
@@ -135,7 +135,7 @@  static int validate_master(const struct ubifs_info *c)
 		goto out;
 	}
 
-	if (c->gc_lnum >= c->leb_cnt || c->gc_lnum < c->main_first) {
+	if (c->gc_lnum != -1 && (c->gc_lnum >= c->leb_cnt || c->gc_lnum < c->main_first)) {
 		err = 7;
 		goto out;
 	}
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 8cdcdc5..0207620 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1260,7 +1260,7 @@  static int mount_ubifs(struct ubifs_info *c)
 
 	init_constants_master(c);
 
-	if ((c->mst_node->flags & cpu_to_le32(UBIFS_MST_DIRTY)) != 0) {
+	if ((c->mst_node->flags & cpu_to_le32(UBIFS_MST_DIRTY)) != 0 || c->gc_lnum == -1) {
 		ubifs_msg("recovery needed");
 		c->need_recovery = 1;
 		if (!mounted_read_only) {