public inbox for ecos-discuss@sourceware.org
 help / color / mirror / Atom feed
* [ECOS] JFFS2 eats memory
@ 2004-07-12 14:42 Øyvind Harboe
  2004-07-13  7:40 ` Andrew Lunn
  2004-07-13  9:30 ` David Woodhouse
  0 siblings, 2 replies; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-12 14:42 UTC (permalink / raw)
  To: ecos-discuss

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

This issue has been discussed before, and although I have a workaround,
I'd dearly like to have it put to bed since it is starting to cause
problems elsewhere in my application:

- My code opens a file for writing with O_TRUNC set, performs
a single write call, closes the file.
- After closing the file, JFFS2 has eaten memory.
- With the attached modifcations to JFFS2, it "only" eats 24 bytes.
- If I unmount and remount JFFS2, no memory is "lost" and JFFS2 works
fine.

Presumably when the raw nodes in the file fragement list are marked as
obsolete, they are no longer required, but are not freed.

Q: Is this fundamentally impossible or a "bad idea" to fix?


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


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

Index: src/fs-ecos.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/fs-ecos.c,v
retrieving revision 1.27
diff -u -w -r1.27 fs-ecos.c
--- src/fs-ecos.c	21 Apr 2004 18:51:21 -0000	1.27
+++ src/fs-ecos.c	12 Jul 2004 14:25:41 -0000
@@ -199,7 +199,9 @@
 	// held where needed for dotdot filepaths)
 	while (this) {
 		next = this->i_cache_next;
-		if (this != i && this->i_count == 0) {
+		if (
+		    //this != i && 
+		    this->i_count == 0) {
 			struct _inode *parent = this->i_parent;
 			if (this->i_cache_next)
 				this->i_cache_next->i_cache_prev = this->i_cache_prev;
@@ -1466,12 +1468,22 @@
 {
 	struct _inode *node = (struct _inode *) fp->f_data;
 
+	// cache values before we destroy the node
+	struct jffs2_sb_info *c = JFFS2_SB_INFO(node->i_sb);
+	struct _inode *root = node->i_sb->s_root;
+
 	D2(printf("jffs2_fo_close\n"));
 
 	jffs2_iput(node);
 
 	fp->f_data = 0;		// zero data pointer
 
+
+	// free as much of cached structures as possible to make sure
+	// that memory usage stays stable and that idle memory usage
+	// is at a minimum.
+	icache_evict(root, node);	
+
 	return ENOERR;
 }
 


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-12 14:42 [ECOS] JFFS2 eats memory Øyvind Harboe
@ 2004-07-13  7:40 ` Andrew Lunn
  2004-07-13  7:53   ` Andrew Lunn
  2004-07-13  7:58   ` Øyvind Harboe
  2004-07-13  9:30 ` David Woodhouse
  1 sibling, 2 replies; 29+ messages in thread
From: Andrew Lunn @ 2004-07-13  7:40 UTC (permalink / raw)
  To: ?yvind Harboe; +Cc: ecos-discuss

On Mon, Jul 12, 2004 at 04:42:11PM +0200, ?yvind Harboe wrote:
> This issue has been discussed before, and although I have a workaround,
> I'd dearly like to have it put to bed since it is starting to cause
> problems elsewhere in my application:
> 
> - My code opens a file for writing with O_TRUNC set, performs
> a single write call, closes the file.
> - After closing the file, JFFS2 has eaten memory.
> - With the attached modifcations to JFFS2, it "only" eats 24 bytes.
> - If I unmount and remount JFFS2, no memory is "lost" and JFFS2 works
> fine.
> 
> Presumably when the raw nodes in the file fragement list are marked as
> obsolete, they are no longer required, but are not freed.
> 
> Q: Is this fundamentally impossible or a "bad idea" to fix?

How much memory are we talking about here in this example?

The cache is there for a reason, so i would not want to
unconditionally disable it. At least you need some CDL to control
between fat & fast and slow & slim.

I would also suggest you consider extending the
CYGNUM_FS_JFFS2_RAW_NODE_REF_CACHE_POOL_SIZE concept to the inode
cache.  You can then control the size of the cache. 

        Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13  7:40 ` Andrew Lunn
@ 2004-07-13  7:53   ` Andrew Lunn
  2004-07-13  8:09     ` Øyvind Harboe
  2004-07-13  8:31     ` Øyvind Harboe
  2004-07-13  7:58   ` Øyvind Harboe
  1 sibling, 2 replies; 29+ messages in thread
From: Andrew Lunn @ 2004-07-13  7:53 UTC (permalink / raw)
  To: ?yvind Harboe, ecos-discuss

> > Presumably when the raw nodes in the file fragement list are marked as
> > obsolete, they are no longer required, but are not freed.

One other thing. You might consider try gettting the garbage collect
code runnig. This might help solve your problem. 

I guess as a proof of concept just call
jffs2_garbage_collect_pass(JFFS2_SB_INFO(jffs2_sb));

This is not strictly correct since you need to add locking to one of
the lists to make this thread safe. I forget which and i've forgotten
where i read about this, but im sure google will find it for you.

        Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13  7:40 ` Andrew Lunn
  2004-07-13  7:53   ` Andrew Lunn
@ 2004-07-13  7:58   ` Øyvind Harboe
  1 sibling, 0 replies; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-13  7:58 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-discuss

On Tue, 2004-07-13 at 09:40, Andrew Lunn wrote:
> On Mon, Jul 12, 2004 at 04:42:11PM +0200, ?yvind Harboe wrote:
> > This issue has been discussed before, and although I have a workaround,
> > I'd dearly like to have it put to bed since it is starting to cause
> > problems elsewhere in my application:
> > 
> > - My code opens a file for writing with O_TRUNC set, performs
> > a single write call, closes the file.
> > - After closing the file, JFFS2 has eaten memory.
> > - With the attached modifcations to JFFS2, it "only" eats 24 bytes.
> > - If I unmount and remount JFFS2, no memory is "lost" and JFFS2 works
> > fine.
> > 
> > Presumably when the raw nodes in the file fragement list are marked as
> > obsolete, they are no longer required, but are not freed.
> > 
> > Q: Is this fundamentally impossible or a "bad idea" to fix?
> 
> How much memory are we talking about here in this example?

24 bytes* number of write() operations for each time a file is
truncated.

Additionally, memory is lost for other operations(e.g. deleting files),
but I don't have any numbers(24 bytes for every time a file is
deleted?).

I'd really like to see that JFFS2 memory usage was constant if the
number of files(and sections within those files) is constant. That way I
could just run my system and measure the maximum usage.

> The cache is there for a reason, so i would not want to
> unconditionally disable it. At least you need some CDL to control
> between fat & fast and slow & slim.

Is JFFS2 faster because it caches raw_node_ref's to deleted nodes?

Sounds strange.

The mount code simply ignores these nodes, which insinuates that caching
these nodes does not help performance.

> I would also suggest you consider extending the
> CYGNUM_FS_JFFS2_RAW_NODE_REF_CACHE_POOL_SIZE concept to the inode
> cache.  You can then control the size of the cache. 

AFAICT, this setting is no help in this case, because all it would do is
to cause JFFS2 operations to fail when running out of pool space instead
of when running out of overall system memory.

Further it would lock a piece of the overall system memory to JFFS2,
making the global memory situation worse.

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



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13  7:53   ` Andrew Lunn
@ 2004-07-13  8:09     ` Øyvind Harboe
  2004-07-13  8:31     ` Øyvind Harboe
  1 sibling, 0 replies; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-13  8:09 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-discuss

On Tue, 2004-07-13 at 09:53, Andrew Lunn wrote:
> > > Presumably when the raw nodes in the file fragement list are marked as
> > > obsolete, they are no longer required, but are not freed.
> 
> One other thing. You might consider try gettting the garbage collect
> code runnig. This might help solve your problem. 

I just took it for a spin: no change.

This is not surprising. 

There is nothing to garbage collect, i.e. there is plenty of free space
on the JFFS2 drive and very few deleted raw nodes.

> I guess as a proof of concept just call
> jffs2_garbage_collect_pass(JFFS2_SB_INFO(jffs2_sb));
> 
> This is not strictly correct since you need to add locking to one of
> the lists to make this thread safe. I forget which and i've forgotten
> where i read about this, but im sure google will find it for you.
> 
>         Andrew
-- 
Øyvind Harboe
http://www.zylin.com



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13  7:53   ` Andrew Lunn
  2004-07-13  8:09     ` Øyvind Harboe
@ 2004-07-13  8:31     ` Øyvind Harboe
  1 sibling, 0 replies; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-13  8:31 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-discuss

This mail explains a lot. I'll try to see if I can't follow his
suggestions.

http://sources.redhat.com/ml/ecos-discuss/2003-10/msg00248.html


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



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-12 14:42 [ECOS] JFFS2 eats memory Øyvind Harboe
  2004-07-13  7:40 ` Andrew Lunn
@ 2004-07-13  9:30 ` David Woodhouse
  2004-07-13  9:49   ` Øyvind Harboe
  1 sibling, 1 reply; 29+ messages in thread
From: David Woodhouse @ 2004-07-13  9:30 UTC (permalink / raw)
  To: Øyvind Harboe; +Cc: ecos-discuss

On Mon, 2004-07-12 at 16:42 +0200, Øyvind Harboe wrote:
> This issue has been discussed before, and although I have a workaround,
> I'd dearly like to have it put to bed since it is starting to cause
> problems elsewhere in my application:
> 
> - My code opens a file for writing with O_TRUNC set, performs
> a single write call, closes the file.
> - After closing the file, JFFS2 has eaten memory.
> - With the attached modifcations to JFFS2, it "only" eats 24 bytes.

That would be a single struct jffs2_raw_node_ref. That's 16 bytes and I
assume the other 8 is allocator overhead -- unless you're on a 64-bit
platform where it is actually 24 bytes.

It wouldn't be hard to cut the jffs2_raw_node_ref down to 12 bytes
instead of 16, btw. There are _many_ of these. See the discussion about
making the commented ref_totlen() macro in nodelist.h actually work in
100% of cases, rather than only 95% as it would at the moment, and then
we could drop the totlen field.

> Presumably when the raw nodes in the file fragement list are marked as
> obsolete, they are no longer required, but are not freed.

The jffs2_raw_node_ref structures _are_ required. You need those in
order to be able to find all the parts of a given inode if/when it gets
opened again.

But the file fragment list, which is held in memory for as long as the
eCos-specific "struct _inode" exists, is not necessarily required.

You've made the eCos code prune its inode cache more aggressively. Be
careful that you don't pessimise the garbage-collection code. It will
often spend a fair bit of time opening an inode (with
jffs2_gc_fetch_inode()), doing some garbage collection, and then
releasing it again (with jffs2_gc_release_inode()).

Now that we have those methods, actually, you could mark only _those_
inodes so that they remain in cache for a little while longer, while
instantly releasing inodes from your icache if they were opened by the
user.

> Q: Is this fundamentally impossible or a "bad idea" to fix?

Not at all. Everything under fs/ecos/ is yours to do with as you see
fit. I'm happy to advise but I'd _really_ like to see someone rewrite
the eCos parts altogether; they still look _far_ too much like a
hacked-up Linux compatibility layer, and they no longer need to.

-- 
dwmw2


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13  9:30 ` David Woodhouse
@ 2004-07-13  9:49   ` Øyvind Harboe
  2004-07-13 10:05     ` David Woodhouse
  2004-07-13 10:08     ` [ECOS] " Thomas Koeller
  0 siblings, 2 replies; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-13  9:49 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ecos-discuss

On Tue, 2004-07-13 at 11:30, David Woodhouse wrote:
> On Mon, 2004-07-12 at 16:42 +0200, Øyvind Harboe wrote:
> > This issue has been discussed before, and although I have a workaround,
> > I'd dearly like to have it put to bed since it is starting to cause
> > problems elsewhere in my application:
> > 
> > - My code opens a file for writing with O_TRUNC set, performs
> > a single write call, closes the file.
> > - After closing the file, JFFS2 has eaten memory.
> > - With the attached modifcations to JFFS2, it "only" eats 24 bytes.
> 
> That would be a single struct jffs2_raw_node_ref. That's 16 bytes and I
> assume the other 8 is allocator overhead -- unless you're on a 64-bit
> platform where it is actually 24 bytes.

Yes.

> It wouldn't be hard to cut the jffs2_raw_node_ref down to 12 bytes
> instead of 16, btw. There are _many_ of these. See the discussion about
> making the commented ref_totlen() macro in nodelist.h actually work in
> 100% of cases, rather than only 95% as it would at the moment, and then
> we could drop the totlen field.

Changing the size of jffs2_raw_node_ref does not help much, since the
problem is that my system runs out of memory since it continously
overwrites existing files, thus filling up the flash with obsoleted
nodes.

> > Presumably when the raw nodes in the file fragement list are marked as
> > obsolete, they are no longer required, but are not freed.
> 
> The jffs2_raw_node_ref structures _are_ required. You need those in
> order to be able to find all the parts of a given inode if/when it gets
> opened again.
> 
> But the file fragment list, which is held in memory for as long as the
> eCos-specific "struct _inode" exists, is not necessarily required.

I'm pretty sure the problem is in the file fragment list...

> You've made the eCos code prune its inode cache more aggressively. Be
> careful that you don't pessimise the garbage-collection code. It will
> often spend a fair bit of time opening an inode (with
> jffs2_gc_fetch_inode()), doing some garbage collection, and then
> releasing it again (with jffs2_gc_release_inode()).

Once/if I get the file fragment memory loss fixed, I'll try to remove it
and run some stress testing for steady state memory usage to see if I
can take out those changes.

> 
> Now that we have those methods, actually, you could mark only _those_
> inodes so that they remain in cache for a little while longer, while
> instantly releasing inodes from your icache if they were opened by the
> user.
> 
> > Q: Is this fundamentally impossible or a "bad idea" to fix?
> 
> Not at all. Everything under fs/ecos/ is yours to do with as you see
> fit. I'm happy to advise but I'd _really_ like to see someone rewrite
> the eCos parts altogether; they still look _far_ too much like a
> hacked-up Linux compatibility layer, and they no longer need to.

Thats a lot more than I want to attempt with my current knowledge about
JFFS2. :-)



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



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13  9:49   ` Øyvind Harboe
@ 2004-07-13 10:05     ` David Woodhouse
  2004-07-13 10:39       ` Øyvind Harboe
  2004-07-13 13:41       ` Øyvind Harboe
  2004-07-13 10:08     ` [ECOS] " Thomas Koeller
  1 sibling, 2 replies; 29+ messages in thread
From: David Woodhouse @ 2004-07-13 10:05 UTC (permalink / raw)
  To: Øyvind Harboe; +Cc: ecos-discuss

On Tue, 2004-07-13 at 11:49 +0200, Øyvind Harboe wrote:
> Changing the size of jffs2_raw_node_ref does not help much, since the
> problem is that my system runs out of memory since it continously
> overwrites existing files, thus filling up the flash with obsoleted
> nodes.

It'll help in a lot of cases. You have a jffs2_raw_node_ref for _all_
data nodes which are physically present on the flash, whether the inodes
to which they belong are opened or not. There can be thousands of these.

In Linux we also use a different allocator, allocating these from
fixed-size slabs so there isn't an 8-byte overhead for each one. 

> I'm pretty sure the problem is in the file fragment list...

What's the problem? You say the problem goes away when you prune the
icache... that implies that there's nothing being _lost_, but you're
just objecting to the fact that your inode cache is doing any caching.

It's keeping the inode around for you in case you open it again
immediately, because it _knows_ the garbage collector tends to exhibit
that kind of behaviour. You made it stop doing that, and your problem
went away, right?

Can you join #mtd on irc.freenode.net?

> Thats a lot more than I want to attempt with my current knowledge about
> JFFS2. :-)

It's not about JFFS2. It's about eCos. The complex parts of the JFFS2
bits are in the core JFFS2 code which you're not expected to touch. IT's
the eCos wrapper around that core code which needs a rewrite :)

-- 
dwmw2


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13  9:49   ` Øyvind Harboe
  2004-07-13 10:05     ` David Woodhouse
@ 2004-07-13 10:08     ` Thomas Koeller
  1 sibling, 0 replies; 29+ messages in thread
From: Thomas Koeller @ 2004-07-13 10:08 UTC (permalink / raw)
  To: ecos-discuss

On Tuesday 13 July 2004 11:49, Øyvind Harboe wrote:

> Changing the size of jffs2_raw_node_ref does not help much, since the
> problem is that my system runs out of memory since it continously
> overwrites existing files, thus filling up the flash with obsoleted
> nodes.

If this is so, then you are actually using only a fraction of the flash
memory you have available. In this case you could reduce the amount of
flash allocated to your JFFS2 file system, so that the garbage collection
would be triggered earlier, and you'd get fewer obsoleted nodes in RAM.

-- 
Thomas Koeller
thomas@koeller.dyndns.org

--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13 10:05     ` David Woodhouse
@ 2004-07-13 10:39       ` Øyvind Harboe
  2004-07-13 13:41       ` Øyvind Harboe
  1 sibling, 0 replies; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-13 10:39 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ecos-discuss

On Tue, 2004-07-13 at 12:05, David Woodhouse wrote:
> On Tue, 2004-07-13 at 11:49 +0200, Øyvind Harboe wrote:
> > Changing the size of jffs2_raw_node_ref does not help much, since the
> > problem is that my system runs out of memory since it continously
> > overwrites existing files, thus filling up the flash with obsoleted
> > nodes.
> 
> It'll help in a lot of cases. You have a jffs2_raw_node_ref for _all_
> data nodes which are physically present on the flash, whether the inodes
> to which they belong are opened or not. There can be thousands of these.
> 
> In Linux we also use a different allocator, allocating these from
> fixed-size slabs so there isn't an 8-byte overhead for each one. 

When I say that reducing size does not "help", I mean that the problem
with memory usage constantly increasing persits. It just takes a bit
longer, e.g. my system falls over in 4 days instead of 1, which in my
case makes no difference.

> 
> > I'm pretty sure the problem is in the file fragment list...
> 
> What's the problem?

I'd like to take this oportunity to say that I'm trying to address this
problem because I need to, not because my eCos/JFFS2 knowledge is up to
the task. 

>  You say the problem goes away when you prune the
> icache...

My problem *almost* goes away with the icache_evict() change.

>  that implies that there's nothing being _lost_, but you're
> just objecting to the fact that your inode cache is doing any caching.
> 
> It's keeping the inode around for you in case you open it again
> immediately, because it _knows_ the garbage collector tends to exhibit
> that kind of behaviour. You made it stop doing that, and your problem
> went away, right?

No, it almost went away. 24 bytes is still lost every time I
overwrite/delete a file

> Can you join #mtd on irc.freenode.net?

I took it for a quick spin, but got some error messages. Perhaps some
firewall issues, etc. I'm not familiar with IRC.

> > Thats a lot more than I want to attempt with my current knowledge about
> > JFFS2. :-)
> 
> It's not about JFFS2. It's about eCos. The complex parts of the JFFS2
> bits are in the core JFFS2 code which you're not expected to touch. IT's
> the eCos wrapper around that core code which needs a rewrite :)

I see.

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



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [ECOS] JFFS2 eats memory
  2004-07-13 10:05     ` David Woodhouse
  2004-07-13 10:39       ` Øyvind Harboe
@ 2004-07-13 13:41       ` Øyvind Harboe
  2004-07-13 23:01         ` [ECOS] " David Woodhouse
  1 sibling, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-13 13:41 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ecos-discuss

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

After your walkthrough if the approach on IRC, I came up with this
patch, which seems to take care of the problem.

If there is a consensus that this is something that would be generally
useful for eCos/JFFS2 users, I'll be happy to clean it up and submit a
patch.


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


[-- Attachment #2: memleakfix.txt --]
[-- Type: text/x-patch, Size: 2041 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	13 Jul 2004 13:35:44 -0000
@@ -549,6 +549,60 @@
 		printk(KERN_WARNING "Short write in obliterating obsoleted node at 0x%08x: %zd\n", ref_offset(ref), retlen);
 		return;
 	}
+
+	// Merge obsolete nodes into its previous node, i.e. always leave
+	// one node behind so as not to screw up ref_totlen()
+	if (ref->next_in_ino!=NULL)
+	  {	
+	    bool moreToDo;
+	    do {
+	      moreToDo=false;	      
+	      struct jffs2_inode_cache *ic;
+	      ic = jffs2_raw_ref_to_ic(ref);
+
+	      // unlink the node and 
+	      struct jffs2_raw_node_ref *raw;
+	      struct jffs2_raw_node_ref *prev=NULL;
+	      raw = ic->nodes;
+	      for (raw = ic->nodes; raw != (void *)ic; raw = raw->next_in_ino) {
+		// if this node *and* the previous *physical node* are obsolete, combine them.
+		if ((prev!=NULL)&&ref_obsolete(raw)) {
+		  // now take take it off the physcial list, unless it is the 
+		  // first node.
+		  struct jffs2_raw_node_ref *t;
+		  struct jffs2_raw_node_ref *phys_prev=NULL;
+		  t=jeb->first_node;
+		  while (t!=NULL) {
+		    if ((phys_prev!=NULL)&&(t==raw)&&ref_obsolete(prev)) {
+		      // take it off the inode list.
+		      prev->next_in_ino=t->next_in_ino;
+		      
+		      // take it off the physical list
+		      phys_prev->next_phys=t->next_phys;
+		      // update last physical entry pointer...
+		      if (jeb->last_node==t) {
+			jeb->last_node=t->next_phys;
+		      }
+		      
+		      // update physical __totlen field.
+		      phys_prev->__totlen+=t->__totlen;
+	      
+	  
+		      jffs2_free_raw_node_ref(raw);
+		      moreToDo=true;
+
+		      break;
+		    }
+		    phys_prev=t;
+		    t=t->next_phys;
+		  }
+		  break;
+		}
+		prev=raw;
+	      }
+	    } while (moreToDo);
+	      }
+	
 }
 
 #if CONFIG_JFFS2_FS_DEBUG > 0


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-13 13:41       ` Øyvind Harboe
@ 2004-07-13 23:01         ` David Woodhouse
  2004-07-14  8:15           ` Øyvind Harboe
  0 siblings, 1 reply; 29+ messages in thread
From: David Woodhouse @ 2004-07-13 23:01 UTC (permalink / raw)
  To: Øyvind Harboe; +Cc: ecos-discuss, linux-mtd

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



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-13 23:01         ` [ECOS] " David Woodhouse
@ 2004-07-14  8:15           ` Øyvind Harboe
  2004-07-19 14:25             ` Øyvind Harboe
  0 siblings, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-14  8:15 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ecos-discuss, linux-mtd

[-- Attachment #1: Type: text/plain, Size: 468 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: 1711 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


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-14  8:15           ` Øyvind Harboe
@ 2004-07-19 14:25             ` Øyvind Harboe
  2004-07-19 15:15               ` David Woodhouse
  0 siblings, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-19 14:25 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ecos-discuss, linux-mtd

[-- 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: 1988 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


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-19 14:25             ` Øyvind Harboe
@ 2004-07-19 15:15               ` David Woodhouse
  2004-07-19 16:32                 ` Øyvind Harboe
  0 siblings, 1 reply; 29+ messages in thread
From: David Woodhouse @ 2004-07-19 15:15 UTC (permalink / raw)
  To: ØyvindHarboe; +Cc: ecos-discuss, linux-mtd

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

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-19 15:15               ` David Woodhouse
@ 2004-07-19 16:32                 ` Øyvind Harboe
  2004-07-20  6:42                   ` David Woodhouse
  0 siblings, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-19 16:32 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ecos-discuss, linux-mtd

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



--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-19 16:32                 ` Øyvind Harboe
@ 2004-07-20  6:42                   ` David Woodhouse
  2004-07-20  7:51                     ` Øyvind Harboe
  2004-07-20 13:46                     ` Øyvind Harboe
  0 siblings, 2 replies; 29+ messages in thread
From: David Woodhouse @ 2004-07-20  6:42 UTC (permalink / raw)
  To: Øyvind Harboe; +Cc: ecos-discuss, linux-mtd

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


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-20  6:42                   ` David Woodhouse
@ 2004-07-20  7:51                     ` Øyvind Harboe
  2004-07-20 14:25                       ` David Woodhouse
  2004-07-20 13:46                     ` Øyvind Harboe
  1 sibling, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-20  7:51 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ecos-discuss, linux-mtd

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



--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-20  6:42                   ` David Woodhouse
  2004-07-20  7:51                     ` Øyvind Harboe
@ 2004-07-20 13:46                     ` Øyvind Harboe
  1 sibling, 0 replies; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-20 13:46 UTC (permalink / raw)
  To: David Woodhouse; +Cc: ecos-discuss, linux-mtd

[-- 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: 5470 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


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-20  7:51                     ` Øyvind Harboe
@ 2004-07-20 14:25                       ` David Woodhouse
  2004-07-20 15:51                         ` Øyvind Harboe
  0 siblings, 1 reply; 29+ messages in thread
From: David Woodhouse @ 2004-07-20 14:25 UTC (permalink / raw)
  To: Øyvind Harboe; +Cc: linux-mtd, ecos-discuss

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


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-20 14:25                       ` David Woodhouse
@ 2004-07-20 15:51                         ` Øyvind Harboe
  2004-07-20 16:08                           ` David Woodhouse
  0 siblings, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-20 15:51 UTC (permalink / raw)
  To: David Woodhouse; +Cc: linux-mtd, ecos-discuss

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



--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-20 15:51                         ` Øyvind Harboe
@ 2004-07-20 16:08                           ` David Woodhouse
  2004-07-20 20:29                             ` Øyvind Harboe
  0 siblings, 1 reply; 29+ messages in thread
From: David Woodhouse @ 2004-07-20 16:08 UTC (permalink / raw)
  To: Øyvind Harboe; +Cc: linux-mtd, ecos-discuss

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


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-20 16:08                           ` David Woodhouse
@ 2004-07-20 20:29                             ` Øyvind Harboe
  2004-07-21  2:28                               ` David Woodhouse
  0 siblings, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-20 20:29 UTC (permalink / raw)
  To: David Woodhouse; +Cc: linux-mtd, ecos-discuss

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



--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-20 20:29                             ` Øyvind Harboe
@ 2004-07-21  2:28                               ` David Woodhouse
  2004-07-21  7:54                                 ` Øyvind Harboe
  0 siblings, 1 reply; 29+ messages in thread
From: David Woodhouse @ 2004-07-21  2:28 UTC (permalink / raw)
  To: Øyvind Harboe; +Cc: linux-mtd, ecos-discuss

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


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-21  2:28                               ` David Woodhouse
@ 2004-07-21  7:54                                 ` Øyvind Harboe
       [not found]                                   ` <1090410703.4280.10.camel@localhost.localdomain>
  0 siblings, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-21  7:54 UTC (permalink / raw)
  To: David Woodhouse; +Cc: linux-mtd, ecos-discuss

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



--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
       [not found]                                   ` <1090410703.4280.10.camel@localhost.localdomain>
@ 2004-07-21 12:50                                     ` Øyvind Harboe
  2004-07-21 16:33                                       ` David Woodhouse
  0 siblings, 1 reply; 29+ messages in thread
From: Øyvind Harboe @ 2004-07-21 12:50 UTC (permalink / raw)
  To: David Woodhouse; +Cc: linux-mtd, ecos-discuss

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



--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] Re: JFFS2 eats memory
  2004-07-21 12:50                                     ` Øyvind Harboe
@ 2004-07-21 16:33                                       ` David Woodhouse
  0 siblings, 0 replies; 29+ messages in thread
From: David Woodhouse @ 2004-07-21 16:33 UTC (permalink / raw)
  To: Øyvind Harboe; +Cc: linux-mtd, ecos-discuss

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


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [ECOS] JFFS2 eats memory
@ 2008-04-08 15:28 Jürgen Lambrecht
  0 siblings, 0 replies; 29+ messages in thread
From: Jürgen Lambrecht @ 2008-04-08 15:28 UTC (permalink / raw)
  To: oyvind.harboe, dwmw2; +Cc: linux-mtd, eCos Discussion

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


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2008-04-08 15:17 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-07-12 14:42 [ECOS] JFFS2 eats memory Øyvind Harboe
2004-07-13  7:40 ` Andrew Lunn
2004-07-13  7:53   ` Andrew Lunn
2004-07-13  8:09     ` Øyvind Harboe
2004-07-13  8:31     ` Øyvind Harboe
2004-07-13  7:58   ` Øyvind Harboe
2004-07-13  9:30 ` David Woodhouse
2004-07-13  9:49   ` Øyvind Harboe
2004-07-13 10:05     ` David Woodhouse
2004-07-13 10:39       ` Øyvind Harboe
2004-07-13 13:41       ` Øyvind Harboe
2004-07-13 23:01         ` [ECOS] " David Woodhouse
2004-07-14  8:15           ` Øyvind Harboe
2004-07-19 14:25             ` Øyvind Harboe
2004-07-19 15:15               ` David Woodhouse
2004-07-19 16:32                 ` Øyvind Harboe
2004-07-20  6:42                   ` David Woodhouse
2004-07-20  7:51                     ` Øyvind Harboe
2004-07-20 14:25                       ` David Woodhouse
2004-07-20 15:51                         ` Øyvind Harboe
2004-07-20 16:08                           ` David Woodhouse
2004-07-20 20:29                             ` Øyvind Harboe
2004-07-21  2:28                               ` David Woodhouse
2004-07-21  7:54                                 ` Øyvind Harboe
     [not found]                                   ` <1090410703.4280.10.camel@localhost.localdomain>
2004-07-21 12:50                                     ` Øyvind Harboe
2004-07-21 16:33                                       ` David Woodhouse
2004-07-20 13:46                     ` Øyvind Harboe
2004-07-13 10:08     ` [ECOS] " Thomas Koeller
2008-04-08 15:28 Jürgen Lambrecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).