[PATCH] Obsolete nodes that are unlinked when possible

[PATCH] Obsolete nodes that are unlinked when possible

[Joakim Tjernlund]

>From d3e51689869fae928f6289307452a698359302ff Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Fri, 16 Mar 2007 16:15:45 +0100
Subject: [PATCH] Obsolete nodes that are unlinked when possible.


Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 fs/jffs2/write.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
index 6717679..57a79a6 100644
--- a/fs/jffs2/write.c
+++ b/fs/jffs2/write.c
@@ -507,8 +507,7 @@ int jffs2_do_unlink(struct jffs2_sb_info *c, struct jffs2_inode_info *dir_f,
 	uint32_t alloclen;
 	int ret;
 
-	if (1 /* alternative branch needs testing */ ||
-	    !jffs2_can_mark_obsolete(c)) {
+	if (!jffs2_can_mark_obsolete(c)) {
 		/* We can't mark stuff obsolete on the medium. We need to write a deletion dirent */
 
 		rd = jffs2_alloc_raw_dirent();
-- 
1.4.4.4

Re: [PATCH] Obsolete nodes that are unlinked when possible

[Joakim Tjernlund]

On Fri, 2007-03-16 at 16:45 +0100, Joakim Tjernlund wrote:
> >From d3e51689869fae928f6289307452a698359302ff Mon Sep 17 00:00:00 2001
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Fri, 16 Mar 2007 16:15:45 +0100
> Subject: [PATCH] Obsolete nodes that are unlinked when possible.
> 
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  fs/jffs2/write.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
> index 6717679..57a79a6 100644
> --- a/fs/jffs2/write.c
> +++ b/fs/jffs2/write.c
> @@ -507,8 +507,7 @@ int jffs2_do_unlink(struct jffs2_sb_info *c, struct jffs2_inode_info *dir_f,
>  	uint32_t alloclen;
>  	int ret;
>  
> -	if (1 /* alternative branch needs testing */ ||
> -	    !jffs2_can_mark_obsolete(c)) {
> +	if (!jffs2_can_mark_obsolete(c)) {
>  		/* We can't mark stuff obsolete on the medium. We need to write a deletion dirent */
>  
>  		rd = jffs2_alloc_raw_dirent();

Ouch, I just discovered that JFFS2_SUMMARY disables
jffs2_can_mark_obsolete(c). Is this really needed for all
obsolete cases?

SUMMARY really implies that a better automatic GC is needed, since your
fs will be littered with deletion entries.

 Jocke

Re: [PATCH] Obsolete nodes that are unlinked when possible

[Artem Bityutskiy]

Hi Joakim,

On Fri, 2007-03-16 at 17:14 +0100, Joakim Tjernlund wrote:
> Ouch, I just discovered that JFFS2_SUMMARY disables
> jffs2_can_mark_obsolete(c). Is this really needed for all
> obsolete cases?
> 
> SUMMARY really implies that a better automatic GC is needed, since
> your
> fs will be littered with deletion entries. 

I am not the author of this stuff, so I probably not the right person
for CC :-)

But it is understandable why they did this - because otherwise it would
need marking corresponding entries in summary as obsolete, and here you
have a problem of an unclean reboot between marking the node and the
summary entry obsolete.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

RE: [PATCH] Obsolete nodes that are unlinked when possible

[Joakim Tjernlund]

 

> -----Original Message-----
> From: Artem Bityutskiy [mailto:dedekind@infradead.org] 
> Sent: den 16 mars 2007 17:27
> To: joakim.tjernlund@transmode.se
> Cc: linux-mtd@lists.infradead.org
> Subject: Re: [PATCH] Obsolete nodes that are unlinked when possible
> 
> Hi Joakim,
> 
> On Fri, 2007-03-16 at 17:14 +0100, Joakim Tjernlund wrote:
> > Ouch, I just discovered that JFFS2_SUMMARY disables
> > jffs2_can_mark_obsolete(c). Is this really needed for all
> > obsolete cases?
> > 
> > SUMMARY really implies that a better automatic GC is needed, since
> > your
> > fs will be littered with deletion entries. 
> 
> I am not the author of this stuff, so I probably not the right person
> for CC :-)

Oh, I took your name from memory. Obviously my memory
can't be trusted anymore :) 

> But it is understandable why they did this - because 
> otherwise it would
> need marking corresponding entries in summary as obsolete, 
> and here you
> have a problem of an unclean reboot between marking the node and the
> summary entry obsolete.

Would bad things happen if an unclean reboot happens or will
you just loose some performance?

 Jocke

PS.
   Do you know whats wrong with the list? Havn't got any mail from
   it for a week.

RE: [PATCH] Obsolete nodes that are unlinked when possible

[Artem Bityutskiy]

On Fri, 2007-03-16 at 18:04 +0100, Joakim Tjernlund wrote:
> Would bad things happen if an unclean reboot happens or will
> you just loose some performance?

Well, probably if you _first_ mark the summary entry obsolete and then
the node itself it would work. The idea that JFFS2 trusts summary and
does not read the actual nodes when scanning.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH] Obsolete nodes that are unlinked when possible

[Ferenc Havasi]

Joakim Tjernlund írta:
> Would bad things happen if an unclean reboot happens or will
> you just loose some performance?
>   
There were many reason to set jffs2_can_mark_obsolete(c) to 0.

As Artem wrote if we would like to enable we have to mark not only the 
node as obsolete but also the summary entry.

It is not too easy because of the followings:
- If you would like to modify the summary node on the flash it breaks 
the CRC of the summary node. A solutions can be: the ACCURATE bit should 
not included in the CRC. An other (worse) solution can be: when we 
obsolate a node we overwrite the summary magic with 0, and at the next 
mount the system will scan the erase block instead of using the summary 
(peformance penalty).
- We also have to take care about not only the already written out 
summary. The summary is written out when the jffs2 close an erase block. 
If you want to mark obsolete a node in the unclosed eraseblock 
(nextblock) you should obsolate it in the collected summary, too. 
However it is not a big task, but also needs code modification and testing.

Ferenc

Re: [PATCH] Obsolete nodes that are unlinked when possible

[Joakim Tjernlund]

On Mon, 2007-03-19 at 11:55 +0100, Ferenc Havasi wrote:
> Joakim Tjernlund írta:
> > Would bad things happen if an unclean reboot happens or will
> > you just loose some performance?
> >   
> There were many reason to set jffs2_can_mark_obsolete(c) to 0.
> 
> As Artem wrote if we would like to enable we have to mark not only the 
> node as obsolete but also the summary entry.
> 
> It is not too easy because of the followings:
> - If you would like to modify the summary node on the flash it breaks 
> the CRC of the summary node. A solutions can be: the ACCURATE bit should 
> not included in the CRC. An other (worse) solution can be: when we 
> obsolate a node we overwrite the summary magic with 0, and at the next 
> mount the system will scan the erase block instead of using the summary 
> (peformance penalty).
> - We also have to take care about not only the already written out 
> summary. The summary is written out when the jffs2 close an erase block. 
> If you want to mark obsolete a node in the unclosed eraseblock 
> (nextblock) you should obsolate it in the collected summary, too. 
> However it is not a big task, but also needs code modification and testing.

I see, however I think it is highly desirable to have both SUMMARY and
mark obsolete support as the FS becomes very dirty quickly on some
systems.

I did some tests with and without SUMMARY support compiled into
the kernel and strangely there wasn't a difference in boot
time on my system, 13 seconds in both cases. The FS image was the same
and had about 110 EB with summary info. Is something broken
wrt. summary support on recent kernels(2.6.20 on Powerpc)?

I have also noted that GC isn't performed unless there is lack of space
on flash. I have also noted that JFFS2 doesn't erase obsolete erase
blocks immediately even though the jffs2 code tries to do this(I known
my 2.4 kernel does this)

 Jocke

RE: [PATCH] Obsolete nodes that are unlinked when possible

[Joakim Tjernlund]

Ping.

> -----Original Message-----
> From: linux-mtd-bounces@lists.infradead.org 
> [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of 
> Joakim Tjernlund
> Sent: den 16 mars 2007 16:45
> To: linux-mtd@lists.infradead.org
> Subject: [PATCH] Obsolete nodes that are unlinked when possible
> 
> >From d3e51689869fae928f6289307452a698359302ff Mon Sep 17 
> 00:00:00 2001
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Fri, 16 Mar 2007 16:15:45 +0100
> Subject: [PATCH] Obsolete nodes that are unlinked when possible.
> 
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  fs/jffs2/write.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
> index 6717679..57a79a6 100644
> --- a/fs/jffs2/write.c
> +++ b/fs/jffs2/write.c
> @@ -507,8 +507,7 @@ int jffs2_do_unlink(struct jffs2_sb_info 
> *c, struct jffs2_inode_info *dir_f,
>  	uint32_t alloclen;
>  	int ret;
>  
> -	if (1 /* alternative branch needs testing */ ||
> -	    !jffs2_can_mark_obsolete(c)) {
> +	if (!jffs2_can_mark_obsolete(c)) {
>  		/* We can't mark stuff obsolete on the medium. 
> We need to write a deletion dirent */
>  
>  		rd = jffs2_alloc_raw_dirent();
> -- 
> 1.4.4.4
> 
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> 
> 

RE: [PATCH] Obsolete nodes that are unlinked when possible

[David Woodhouse]

On Thu, 2007-04-19 at 21:34 +0200, Joakim Tjernlund wrote:
> Ping. 

Sorry for taking so long to look at this. Did you test this without
SUMMARY support? 

I agree that we ought to make the summary code _not_ imply
!jffs2_can_mark_obsolete(). There's a few things I want to change about
the summary code -- the nodes are currently about twice as large as they
need to be, too. I'll look at that soon.

-- 
dwmw2

RE: [PATCH] Obsolete nodes that are unlinked when possible

[Joakim Tjernlund]

> -----Original Message-----
> From: David Woodhouse [mailto:dwmw2@infradead.org] 
> Sent: den 20 april 2007 23:14
> To: Joakim Tjernlund
> Cc: linux-mtd@lists.infradead.org
> Subject: RE: [PATCH] Obsolete nodes that are unlinked when possible
> 
> On Thu, 2007-04-19 at 21:34 +0200, Joakim Tjernlund wrote:
> > Ping. 
> 
> Sorry for taking so long to look at this. Did you test this without
> SUMMARY support?

hmm, not sure. I think so, but not much. The idea was that to get
some testing, you need to enable its use in MTD git. 
 
> 
> I agree that we ought to make the summary code _not_ imply
> !jffs2_can_mark_obsolete(). There's a few things I want to 
> change about
> the summary code -- the nodes are currently about twice as 
> large as they
> need to be, too. I'll look at that soon.

Rigth, but more important is that the GC procedure needs to GC
more aggressively, otherwise you will end up with a very fragmented
FS in some cases.

ATM I am doing board bringup and won't be able to spend any time
on JFFS2.

  Jocke