Re: JFFS2 eats memory

Re: JFFS2 eats memory

[David Woodhouse]

On Tue, 2004-07-13 at 15:41 +0200, Øyvind Harboe wrote:
> After your walkthrough if the approach on IRC, I came up with this
> patch, which seems to take care of the problem.

I can't work out what that's doing. Does it do something like this?


Index: nodemgmt.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/nodemgmt.c,v
retrieving revision 1.107
diff -u -p -r1.107 nodemgmt.c
--- nodemgmt.c	26 Nov 2003 15:30:58 -0000	1.107
+++ nodemgmt.c	13 Jul 2004 23:00:48 -0000
@@ -549,6 +549,46 @@ void jffs2_mark_node_obsolete(struct jff
 		printk(KERN_WARNING "Short write in obliterating obsoleted node at 0x%08x: %zd\n", ref_offset(ref), retlen);
 		return;
 	}
+
+	/* Nodes which have been marked obsolete no longer need to be
+	   associated with any inode. Remove them from the per-inode list */
+	if (ref->next_in_ino) {
+		struct jffs2_inode_cache *ic;
+		struct jffs2_raw_node_ref **p;
+
+		ic = jffs2_raw_ref_to_ic(ref);
+		for (p = &ic->nodes; (*p) != ref; p = &((*p)->next_in_ino))
+			;
+
+		*p = ref->next_in_ino;
+		ref->next_in_ino = NULL;
+	}
+
+	/* Merge with the next node in the physical list, if there is one
+	   and if it's also obsolete */
+	if (ref->next_phys && ref_obsolete(ref->next_phys)) {
+		struct jffs2_raw_node_ref *n = ref->next_phys;
+
+		ref->__totlen += n->__totlen;
+		ref->next_phys = n->next_phys;
+		BUG_ON(n->next_in_ino);
+		jffs2_free_raw_node_ref(n);
+	}
+
+	/* Also merge with the previous node in the list, if there is one
+	   and it that one is obsolete */
+	if (ref != jeb->first_node) {
+		struct jffs2_raw_node_ref *p = jeb->first_node;
+
+		while (p->next_phys != ref)
+			p = p->next_phys;
+
+		if (ref_obsolete(p)) {
+			p->__totlen += ref->__totlen;
+			p->next_phys = ref->next_phys;
+			jffs2_free_raw_node_ref(ref);
+		}
+	}
 }
 
 #if CONFIG_JFFS2_FS_DEBUG > 0


-- 
dwmw2

Re: JFFS2 eats memory

[Øyvind Harboe]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

On Wed, 2004-07-14 at 01:01, David Woodhouse wrote:
> On Tue, 2004-07-13 at 15:41 +0200, Øyvind Harboe wrote:
> > After your walkthrough if the approach on IRC, I came up with this
> > patch, which seems to take care of the problem.
> 
> I can't work out what that's doing. Does it do something like this?

Yes.

- I made a small fix: update jeb->last_node
- Ran some tests and it appears to do the job nicely on my rocket.


-- 
Øyvind Harboe
http://www.zylin.com


[-- Attachment #2: memfix.txt --]
[-- Type: text/x-patch, Size: 1710 bytes --]

Index: nodemgmt.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/nodemgmt.c,v
retrieving revision 1.6
diff -w -u -r1.6 nodemgmt.c
--- nodemgmt.c	11 Dec 2003 23:33:54 -0000	1.6
+++ nodemgmt.c	14 Jul 2004 08:12:21 -0000
@@ -549,6 +549,50 @@
 		printk(KERN_WARNING "Short write in obliterating obsoleted node at 0x%08x: %zd\n", ref_offset(ref), retlen);
 		return;
 	}
+
+	/* Nodes which have been marked obsolete no longer need to be
+	   associated with any inode. Remove them from the per-inode list */
+	if (ref->next_in_ino) {
+		struct jffs2_inode_cache *ic;
+		struct jffs2_raw_node_ref **p;
+
+		ic = jffs2_raw_ref_to_ic(ref);
+		for (p = &ic->nodes; (*p) != ref; p = &((*p)->next_in_ino))
+			;
+
+		*p = ref->next_in_ino;
+		ref->next_in_ino = NULL;
+	}
+
+	/* Merge with the next node in the physical list, if there is one
+	   and if it's also obsolete */
+	if (ref->next_phys && ref_obsolete(ref->next_phys)) {
+		struct jffs2_raw_node_ref *n = ref->next_phys;
+
+		ref->__totlen += n->__totlen;
+		/* we don't need to check jeb->last_node */
+		ref->next_phys = n->next_phys;
+		BUG_ON(n->next_in_ino);
+		jffs2_free_raw_node_ref(n);
+	}
+
+	/* Also merge with the previous node in the list, if there is one
+	   and it that one is obsolete */
+	if (ref != jeb->first_node) {
+		struct jffs2_raw_node_ref *p = jeb->first_node;
+
+		while (p->next_phys != ref)
+			p = p->next_phys;
+
+		if (ref_obsolete(p)) {
+			p->__totlen += ref->__totlen;
+			if (jeb->last_node == ref) {
+			  jeb->last_node = p;
+			}
+			p->next_phys = ref->next_phys;
+			jffs2_free_raw_node_ref(ref);
+		}
+	}
 }
 
 #if CONFIG_JFFS2_FS_DEBUG > 0

Re: JFFS2 eats memory

[Øyvind Harboe]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Wed, 2004-07-14 at 10:15, Øyvind Harboe wrote:
> On Wed, 2004-07-14 at 01:01, David Woodhouse wrote:
> > On Tue, 2004-07-13 at 15:41 +0200, Øyvind Harboe wrote:
> > > After your walkthrough if the approach on IRC, I came up with this
> > > patch, which seems to take care of the problem.
> > 
> > I can't work out what that's doing. Does it do something like this?
> 
> Yes.
> 
> - I made a small fix: update jeb->last_node
> - Ran some tests and it appears to do the job nicely on my rocket.

It crashes during garbage collect.

How does this fix look?

-- 
Øyvind Harboe
http://www.zylin.com


[-- Attachment #2: memfixgc.txt --]
[-- Type: text/x-patch, Size: 1987 bytes --]

Index: nodemgmt.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/nodemgmt.c,v
retrieving revision 1.6
diff -u -w -r1.6 nodemgmt.c
--- nodemgmt.c	11 Dec 2003 23:33:54 -0000	1.6
+++ nodemgmt.c	19 Jul 2004 14:15:48 -0000
@@ -549,6 +549,54 @@
 		printk(KERN_WARNING "Short write in obliterating obsoleted node at 0x%08x: %zd\n", ref_offset(ref), retlen);
 		return;
 	}
+
+	/* Nodes which have been marked obsolete no longer need to be
+	   associated with any inode. Remove them from the per-inode list */
+	if (ref->next_in_ino) {
+		struct jffs2_inode_cache *ic;
+		struct jffs2_raw_node_ref **p;
+
+		ic = jffs2_raw_ref_to_ic(ref);
+		for (p = &ic->nodes; (*p) != ref; p = &((*p)->next_in_ino))
+			;
+
+		*p = ref->next_in_ino;
+		ref->next_in_ino = NULL;
+	}
+
+	/* try to free memory unless a garbage collect is in progress. */
+	if (jeb->gc_node==NULL) {
+	  /* Merge with the next node in the physical list, if there is one
+	     and if it's also obsolete. */
+	  if (ref->next_phys && ref_obsolete(ref->next_phys) ) {
+	    struct jffs2_raw_node_ref *n = ref->next_phys;
+	    if (jeb->gc_node != n) {
+	      ref->__totlen += n->__totlen;
+	      /* we don't need to check jeb->last_node */
+	      ref->next_phys = n->next_phys;
+	      BUG_ON(n->next_in_ino);
+	      jffs2_free_raw_node_ref(n);
+	    }
+	  }
+	  
+	  /* Also merge with the previous node in the list, if there is one
+	     and that one is obsolete and it is not currently being garabage collected */
+	  if (ref != jeb->first_node ) {
+	    struct jffs2_raw_node_ref *p = jeb->first_node;
+	    
+	    while (p->next_phys != ref)
+	      p = p->next_phys;
+	    
+	    if (ref_obsolete(p) ) {
+	      p->__totlen += ref->__totlen;
+	      if (jeb->last_node == ref) {
+		jeb->last_node = p;
+	      }
+	      p->next_phys = ref->next_phys;
+	      jffs2_free_raw_node_ref(ref);
+	    }
+	  }
+	}
 }
 
 #if CONFIG_JFFS2_FS_DEBUG > 0

Re: JFFS2 eats memory

[David Woodhouse]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF8, Size: 407 bytes --]

On Mon, 19 Jul 2004, [ISO-8859-1] ØyvindHarboe wrote:

> It crashes during garbage collect.
> 
> How does this fix look?

Suboptimal. You refrain from freeing if(jeb->gc_node) but that's going to 
mean that every time GC frees such a node you're not claiming the memory 
back? Can't you adjust gc_node instead?

Standard comments about 8-char indentation and "==NULL" being horrid also 
apply :)

-- 
dwmw2

Re: JFFS2 eats memory

[Øyvind Harboe]

man, 19.07.2004 kl. 16.24 skrev David Woodhouse:
> On Mon, 19 Jul 2004, [ISO-8859-1] yvindHarboe wrote:
> 
> > It crashes during garbage collect.
> > 
> > How does this fix look?
> 
> Suboptimal. You refrain from freeing if(jeb->gc_node) but that's going to 
> mean that every time GC frees such a node you're not claiming the memory 
> back? 

I suppose.

At this point I'm just trying to find a fix that is correct. 

> Can't you adjust gc_node instead?

I don't know.

What should I adjust it to?

> Standard comments about 8-char indentation 

I need to beat emacs into submission at some point...

> and "==NULL" being horrid also apply :)

I have a database over all the formatting preferences of all the
different projects and source codes I'm working on, and your profile
still needs a bit of tweaking. ;-)

-- 
Øyvind Harboe
http://www.zylin.com

Re: JFFS2 eats memory

[David Woodhouse]

On Mon, 2004-07-19 at 17:15 +0200, Øyvind Harboe wrote:
> At this point I'm just trying to find a fix that is correct. 

Well yes, but this _is_ a performance optimisation you're doing :)

> > Can't you adjust gc_node instead?
> 
> I don't know.
> 
> What should I adjust it to?

gc_node is the next node to be garbage-collected. We don't need to
garbage-collect nodes which are already obsolete. So if you're freeing
the object which is currently pointed to by jeb->gc_node, you can just
make gc_node point to the next_phys node which you're _not_ freeing.

-- 
dwmw2

Re: JFFS2 eats memory

[Øyvind Harboe]

On Tue, 2004-07-20 at 03:10, David Woodhouse wrote:
> On Mon, 2004-07-19 at 17:15 +0200, Øyvind Harboe wrote:
> > At this point I'm just trying to find a fix that is correct. 
>
> Well yes, but this _is_ a performance optimisation you're doing :)

Granted.

> > > Can't you adjust gc_node instead?
> > 
> > I don't know.
> > 
> > What should I adjust it to?
> 
> gc_node is the next node to be garbage-collected. We don't need to
> garbage-collect nodes which are already obsolete. So if you're freeing
> the object which is currently pointed to by jeb->gc_node, you can just
> make gc_node point to the next_phys node which you're _not_ freeing.

What if gc_node points to the last node?

-- 
Øyvind Harboe
http://www.zylin.com

Re: JFFS2 eats memory

[David Woodhouse]

On Tue, 2004-07-20 at 08:41 +0200, Øyvind Harboe wrote:
> What if gc_node points to the last node?

Then you just finished garbage collecting the eraseblock in question.
You should have a single node which is now first_node and last_node, and
which is obsolete. You can just set gc_node to NULL.

-- 
dwmw2

Re: JFFS2 eats memory

[Øyvind Harboe]

tir, 20.07.2004 kl. 15.45 skrev David Woodhouse:
> On Tue, 2004-07-20 at 08:41 +0200, Øyvind Harboe wrote:
> > What if gc_node points to the last node?
> 
> Then you just finished garbage collecting the eraseblock in question.
> You should have a single node which is now first_node and last_node, and
> which is obsolete. You can just set gc_node to NULL.

Setting gc_node to NULL crashed when I tried it.

-- 
Øyvind Harboe
http://www.zylin.com

Re: JFFS2 eats memory

[David Woodhouse]

On Tue, 2004-07-20 at 17:28 +0200, Øyvind Harboe wrote:
> tir, 20.07.2004 kl. 15.45 skrev David Woodhouse:
> > On Tue, 2004-07-20 at 08:41 +0200, Øyvind Harboe wrote:
> > > What if gc_node points to the last node?
> > 
> > Then you just finished garbage collecting the eraseblock in question.
> > You should have a single node which is now first_node and last_node, and
> > which is obsolete. You can just set gc_node to NULL.
> 
> Setting gc_node to NULL crashed when I tried it.

Crashed how? It should be trivially fixable.

-- 
dwmw2

Re: JFFS2 eats memory

[Øyvind Harboe]

tir, 20.07.2004 kl. 17.54 skrev David Woodhouse:
> On Tue, 2004-07-20 at 17:28 +0200, Øyvind Harboe wrote:
> > tir, 20.07.2004 kl. 15.45 skrev David Woodhouse:
> > > On Tue, 2004-07-20 at 08:41 +0200, Øyvind Harboe wrote:
> > > > What if gc_node points to the last node?
> > > 
> > > Then you just finished garbage collecting the eraseblock in question.
> > > You should have a single node which is now first_node and last_node, and
> > > which is obsolete. You can just set gc_node to NULL.
> > 
> > Setting gc_node to NULL crashed when I tried it.

How about my latest version which sets gc_node to the previous node?

> Crashed how? It should be trivially fixable.

I caught it in gc.c where at some point the code assumes that gc_node
does not change beneath it. Don't remember.

-- 
Øyvind Harboe
http://www.zylin.com

Re: JFFS2 eats memory

[David Woodhouse]

On Tue, 2004-07-20 at 20:52 +0200, Øyvind Harboe wrote:
> I caught it in gc.c where at some point the code assumes that gc_node
> does not change beneath it. Don't remember.

Hmmm. That sounds like it could break anyway. Can you be more specific?

Also, memset the raw_node_ref to 0xdeadbeef before you free it. (Or run
with slab poisoning enabled in Linux). We should go through the code and
make sure manually that nothing's going to dereference a pointer to the
old node after it's freed, but the poisoning is a quick and useful
debugging aid.

-- 
dwmw2

Re: JFFS2 eats memory

[Øyvind Harboe]

On Wed, 2004-07-21 at 00:08, David Woodhouse wrote:
> On Tue, 2004-07-20 at 20:52 +0200, Øyvind Harboe wrote:
> > I caught it in gc.c where at some point the code assumes that gc_node
> > does not change beneath it. Don't remember.
> 
> Hmmm. That sounds like it could break anyway. Can you be more specific?

1. Set jeb->gc_node = NULL; at the end of jffs2_mark_node_obsolete(); 

2. fire up the debugger and start writing to the JFFS2 disk.

3. See below...

in gc.c:

	240	
-	241		if (!raw->next_in_ino) {
 	242			/* Inode-less node. Clean marker, snapshot or something like
that */
 	243			/* FIXME: If it's something that needs to be copied, including
something
 	244			   we don't grok that has JFFS2_NODETYPE_RWCOMPAT_COPY, we
should do so */
 	245			spin_unlock(&c->erase_completion_lock);
-	246			jffs2_mark_node_obsolete(c, raw);
-	247			up(&c->alloc_sem);
-	248			goto eraseit_lock;
 	249		}
 	250	

----- Here raw == NULL, hence jffs2_raw_ref_to_ic() crashes.

-	251		ic = jffs2_raw_ref_to_ic(raw);


> Also, memset the raw_node_ref to 0xdeadbeef before you free it. (Or run
> with slab poisoning enabled in Linux). We should go through the code and
> make sure manually that nothing's going to dereference a pointer to the
> old node after it's freed, but the poisoning is a quick and useful
> debugging aid.

eCos, which I'm using, has this facility built in:
CYGDBG_MEMALLOC_ALLOCATOR_DLMALLOC_DEBUG 


-- 
Øyvind Harboe
http://www.zylin.com

Re: JFFS2 eats memory

[David Woodhouse]

On Wed, 2004-07-21 at 08:25 +0200, Øyvind Harboe wrote:

> in gc.c:

> -	241		if (!raw->next_in_ino) {

> -	251		ic = jffs2_raw_ref_to_ic(raw);

Hmmm. Surely you shouldn't be able to get to those in the case where
gc_node is NULL? You should hit the condition at line 218 because
jeb->user_size should be zero.

Remember, gc_node is the placemarker for the garbage-collector which is
busily obsoleting every node in this block so that the block can be
erased and returned to the free pool. If you were freeing a node, and
there was no 'next' node when you did so, that must have meant you got
to the end of the eraseblock, surely?

Obviously I'm wrong -- you have empirical evidence. But why?

PS. Will somebody please kick Beat Morf <beat.morf@duagon.ch> off the
eCos list? He has an extremely broken autoresponder -- it's replying to
the From: address in the mail instead of the SMTP reverse-path, it's
replying with non-error message status so that it can cause mail loops,
and it's not even rate-limited. I get a response for every mail I send.

-- 
dwmw2

Re: JFFS2 eats memory

[Øyvind Harboe]

On Wed, 2004-07-21 at 13:51, David Woodhouse wrote:
> On Wed, 2004-07-21 at 08:25 +0200, Øyvind Harboe wrote:
> 
> > in gc.c:
> 
> > -	241		if (!raw->next_in_ino) {
> 
> > -	251		ic = jffs2_raw_ref_to_ic(raw);
> 
> Hmmm. Surely you shouldn't be able to get to those in the case where
> gc_node is NULL? You should hit the condition at line 218 because
> jeb->user_size should be zero.
> 
> Remember, gc_node is the placemarker for the garbage-collector which is
> busily obsoleting every node in this block so that the block can be
> erased and returned to the free pool. If you were freeing a node, and
> there was no 'next' node when you did so, that must have meant you got
> to the end of the eraseblock, surely?
> 
> Obviously I'm wrong -- you have empirical evidence. But why?

I have no idea. It is easy to reproduce in the fashion I described and
this problem is complex enough that it takes a JFFS2 export a couple of
rounds in the debugger to sort out. At this point I think little would
be gained by me pursuing it.

But. What about my fix where I set jeb->gc_node to the previous element?
Isn't that a better solution in any case?

> PS. Will somebody please kick Beat Morf <beat.morf@duagon.ch> off the
> eCos list?

He will be punished at least. "autorespond = valid email address spack
ack support" :-)

-- 
Øyvind Harboe
http://www.zylin.com

Re: JFFS2 eats memory

[David Woodhouse]

On Wed, 2004-07-21 at 14:03 +0200, Øyvind Harboe wrote:
> > Obviously I'm wrong -- you have empirical evidence. But why?
> 
> I have no idea. It is easy to reproduce in the fashion I described and
> this problem is complex enough that it takes a JFFS2 export a couple of
> rounds in the debugger to sort out. At this point I think little would
> be gained by me pursuing it.

OK, I'll look at it when I get home from OLS.

> But. What about my fix where I set jeb->gc_node to the previous element?
> Isn't that a better solution in any case?

I don't like the idea that the gc_node could go _backwards_ very much.
Although it should be fine really I'd like to know why moving it
forwards (in particular to NULL) was causing the code to break. 

If my assumptions about the behaviour were flawed, I'd like to know
precisely why. It might affect the other alternative too, just not as
obviously.

-- 
dwmw2

Re: JFFS2 eats memory

[Øyvind Harboe]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

> gc_node is the next node to be garbage-collected. We don't need to
> garbage-collect nodes which are already obsolete. So if you're freeing
> the object which is currently pointed to by jeb->gc_node, you can just
> make gc_node point to the next_phys node which you're _not_ freeing.

I modified the code to have gc continue on the previous node(the next
node does not always exist).



-- 
Øyvind Harboe
http://www.zylin.com


[-- Attachment #2: gcjffsmemfix.txt --]
[-- Type: text/plain, Size: 5469 bytes --]

? gcmemfix.txt
? memfix.txt
? memfixgc.txt
? memleakfix.txt
? mutex.txt
? oyvind@84.234.138.230
Index: build.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/build.c,v
retrieving revision 1.5
diff -w -u -r1.5 build.c
--- build.c	20 Nov 2003 16:52:36 -0000	1.5
+++ build.c	20 Jul 2004 13:35:11 -0000
@@ -259,6 +259,14 @@
 
 	c->resv_blocks_write = c->resv_blocks_deletion + (size / c->sector_size);
 
+	// If the flash disk is smaller than resv_blocks_write, then we 
+	// allow writing to the disk anyway. The flash disk is then most likely
+	// being used as write once - read many medimum, e.g. configuration of 
+	// static paramters.
+	if (c->resv_blocks_write * c->sector_size > c->flash_size) {
+	  c->resv_blocks_write = 0; 
+	}
+
 	/* When do we let the GC thread run in the background */
 
 	c->resv_blocks_gctrigger = c->resv_blocks_write + 1;
Index: dir-ecos.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/dir-ecos.c,v
retrieving revision 1.5
diff -w -u -r1.5 dir-ecos.c
--- dir-ecos.c	11 Dec 2003 23:33:54 -0000	1.5
+++ dir-ecos.c	20 Jul 2004 13:35:11 -0000
@@ -48,9 +48,11 @@
 	up(&dir_f->sem);
 	if (ino) {
 		inode = jffs2_iget(dir_i->i_sb, ino);
-		if (!inode) {
+		if (IS_ERR(inode)) {
 			printk("jffs2_iget() failed for ino #%u\n", ino);
-			return (ERR_PTR(-EIO));
+			// NOTE! inode is *not* a pointer here, but an
+			// error code we propagate.
+			return inode;
 		}
 	}
 
Index: erase.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/erase.c,v
retrieving revision 1.6
diff -w -u -r1.6 erase.c
--- erase.c	11 Dec 2003 23:33:54 -0000	1.6
+++ erase.c	20 Jul 2004 13:35:11 -0000
@@ -365,11 +365,12 @@
 		jeb->dirty_size = 0;
 		jeb->wasted_size = 0;
 	} else {
-		struct jffs2_unknown_node marker = {
-			.magic =	cpu_to_je16(JFFS2_MAGIC_BITMASK),
-			.nodetype =	cpu_to_je16(JFFS2_NODETYPE_CLEANMARKER),
-			.totlen =	cpu_to_je32(c->cleanmarker_size)
-		};
+		
+		struct jffs2_unknown_node marker;
+		memset(&marker, 0, sizeof(marker));
+		marker.magic =	cpu_to_je16(JFFS2_MAGIC_BITMASK);
+		marker.nodetype =	cpu_to_je16(JFFS2_NODETYPE_CLEANMARKER);
+		marker.totlen =	cpu_to_je32(c->cleanmarker_size);
 
 		marker.hdr_crc = cpu_to_je32(crc32(0, &marker, sizeof(struct jffs2_unknown_node)-4));
 
Index: fs-ecos.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/fs-ecos.c,v
retrieving revision 1.27
diff -w -u -r1.27 fs-ecos.c
--- fs-ecos.c	21 Apr 2004 18:51:21 -0000	1.27
+++ fs-ecos.c	20 Jul 2004 13:35:12 -0000
@@ -302,7 +302,7 @@
 	d = jffs2_lookup(dir, name, namelen);
 	D2(printf("find_entry got dir = %x\n", d));
 
-	if (d == NULL)
+	if ((d==NULL)||IS_ERR(d))
 		return ENOENT;
 
 	// If it's a new directory inode, increase refcount on its parent
Index: gc.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/gc.c,v
retrieving revision 1.7
diff -w -u -r1.7 gc.c
--- gc.c	1 Apr 2004 03:17:57 -0000	1.7
+++ gc.c	20 Jul 2004 13:35:13 -0000
@@ -358,10 +358,10 @@
 	spin_unlock(&c->inocache_lock);
 
 	f = jffs2_gc_fetch_inode(c, inum, nlink);
-	if (IS_ERR(f))
-		return PTR_ERR(f);
-	if (!f)
-		return 0;
+	if (!f||IS_ERR(f)) {
+	  up(&c->alloc_sem);
+		return f;
+	}
 
 	ret = jffs2_garbage_collect_live(c, jeb, raw, f);
 
Index: nodemgmt.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/nodemgmt.c,v
retrieving revision 1.6
diff -w -u -r1.6 nodemgmt.c
--- nodemgmt.c	11 Dec 2003 23:33:54 -0000	1.6
+++ nodemgmt.c	20 Jul 2004 13:35:13 -0000
@@ -549,6 +549,59 @@
 		printk(KERN_WARNING "Short write in obliterating obsoleted node at 0x%08x: %zd\n", ref_offset(ref), retlen);
 		return;
 	}
+
+	/* Nodes which have been marked obsolete no longer need to be
+	   associated with any inode. Remove them from the per-inode list */
+	if (ref->next_in_ino) {
+		struct jffs2_inode_cache *ic;
+		struct jffs2_raw_node_ref **p;
+
+		ic = jffs2_raw_ref_to_ic(ref);
+		for (p = &ic->nodes; (*p) != ref; p = &((*p)->next_in_ino))
+			;
+
+		*p = ref->next_in_ino;
+		ref->next_in_ino = NULL;
+	}
+
+
+	/* Merge with the next node in the physical list, if there is one
+	   and if it's also obsolete. */
+	if (ref->next_phys && ref_obsolete(ref->next_phys) ) {
+		struct jffs2_raw_node_ref *n = ref->next_phys;
+		
+		ref->__totlen += n->__totlen;
+		/* we don't need to check jeb->last_node */
+		ref->next_phys = n->next_phys;
+		if (jeb->gc_node == n) {
+			/* gc will be happy continuing gc on this node */
+			jeb->gc_node=ref;
+		}
+		BUG_ON(n->next_in_ino);
+		jffs2_free_raw_node_ref(n);
+	}
+	
+	/* Also merge with the previous node in the list, if there is one
+	   and that one is obsolete */
+	if (ref != jeb->first_node ) {
+		struct jffs2_raw_node_ref *p = jeb->first_node;
+		
+		while (p->next_phys != ref)
+			p = p->next_phys;
+		
+		if (ref_obsolete(p) ) {
+			p->__totlen += ref->__totlen;
+			if (jeb->last_node == ref) {
+				jeb->last_node = p;
+			}
+			if (jeb->gc_node == ref) {
+				/* gc will be happy continuing gc on this node */
+				jeb->gc_node=p;
+			}
+			p->next_phys = ref->next_phys;
+			jffs2_free_raw_node_ref(ref);
+		}
+	}
 }
 
 #if CONFIG_JFFS2_FS_DEBUG > 0

JFFS2 eats memory

[Jürgen Lambrecht]

Hello Oyvind and David,

in 2004, you talked about "JFFS2 eats memory". But the thread was 
stopped when David went to "OLS".
I have the same problem now, see my last mail about: JFFS2 "eats RAM" 
per file in flash.
Did you continue on this?
Or should I look to jffs3 or ubifs?

Kind regards,

-- 
Jürgen Lambrecht
R&D Engineer
Televic Transport Systems
http://www.televic.com
Televic NV / SA (main office)  	
Leo Bekaertlaan 1
B-8870 Izegem
Tel: +32 (0)51 303045
Fax: +32 (0)51 310670