* [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: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-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-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 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 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
[parent not found: <1090410703.4280.10.camel@localhost.localdomain>]
* [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] 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
* 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
* [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).