[PATCH] JFFS2: Erase max 3 EB's in jffs2_write_super()

[PATCH] JFFS2: Erase max 3 EB's in jffs2_write_super()

[Joakim Tjernlund]

Deleting big files in a JFFS2 root FS and the rebooting makes
the system hang until all freed EB's has been erased which
can take several minutes.
Fix the hanging by making jffs2_write_super() erase max 3 EB's
at a time. If there are more EB's to erase afterwards, reset the
s_dirty flag.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---

 I hope the list_empty calls doesn't need locking. If
 so, it can easily be added.

 fs/jffs2/super.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 9a80e8e..10393e0 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -64,7 +64,10 @@ static void jffs2_write_super(struct super_block *sb)
 	if (!(sb->s_flags & MS_RDONLY)) {
 		D1(printk(KERN_DEBUG "jffs2_write_super()\n"));
 		jffs2_garbage_collect_trigger(c);
-		jffs2_erase_pending_blocks(c, 0);
+		jffs2_erase_pending_blocks(c, 3);
+		if (!list_empty(&c->erase_complete_list) ||
+		    !list_empty(&c->erase_pending_list))
+			sb->s_dirt = 1;
 		jffs2_flush_wbuf_gc(c, 0);
 	}
 
-- 
1.6.4.4

Re: [PATCH] JFFS2: Erase max 3 EB's in jffs2_write_super()

[Joakim Tjernlund]

Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote on 2010/02/02 10:40:00:
>
> Deleting big files in a JFFS2 root FS and the rebooting makes
> the system hang until all freed EB's has been erased which
> can take several minutes.
> Fix the hanging by making jffs2_write_super() erase max 3 EB's
> at a time. If there are more EB's to erase afterwards, reset the
> s_dirty flag.
>
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>
>  I hope the list_empty calls doesn't need locking. If
>  so, it can easily be added.
>
>  fs/jffs2/super.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 9a80e8e..10393e0 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -64,7 +64,10 @@ static void jffs2_write_super(struct super_block *sb)
>     if (!(sb->s_flags & MS_RDONLY)) {

I wonder if not MS_ACTIVE should be included in the above test too?

       Jocke

Re: [PATCH] JFFS2: Erase max 3 EB's in jffs2_write_super()

[Joakim Tjernlund]

Ping?

Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote on 2010/02/02 10:40:00:
>
> Deleting big files in a JFFS2 root FS and the rebooting makes
> the system hang until all freed EB's has been erased which
> can take several minutes.
> Fix the hanging by making jffs2_write_super() erase max 3 EB's
> at a time. If there are more EB's to erase afterwards, reset the
> s_dirty flag.
>
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>
>  I hope the list_empty calls doesn't need locking. If
>  so, it can easily be added.
>
>  fs/jffs2/super.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 9a80e8e..10393e0 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -64,7 +64,10 @@ static void jffs2_write_super(struct super_block *sb)
>     if (!(sb->s_flags & MS_RDONLY)) {
>        D1(printk(KERN_DEBUG "jffs2_write_super()\n"));
>        jffs2_garbage_collect_trigger(c);
> -      jffs2_erase_pending_blocks(c, 0);
> +      jffs2_erase_pending_blocks(c, 3);
> +      if (!list_empty(&c->erase_complete_list) ||
> +          !list_empty(&c->erase_pending_list))
> +         sb->s_dirt = 1;
>        jffs2_flush_wbuf_gc(c, 0);
>     }
>
> --
> 1.6.4.4
>

Re: [PATCH] JFFS2: Erase max 3 EB's in jffs2_write_super()

[Artem Bityutskiy]

On Tue, 2010-02-02 at 10:40 +0100, Joakim Tjernlund wrote:
> Deleting big files in a JFFS2 root FS and the rebooting makes
> the system hang until all freed EB's has been erased which
> can take several minutes.
> Fix the hanging by making jffs2_write_super() erase max 3 EB's
> at a time. If there are more EB's to erase afterwards, reset the
> s_dirty flag.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
> 
>  I hope the list_empty calls doesn't need locking. If
>  so, it can easily be added.
> 
>  fs/jffs2/super.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 9a80e8e..10393e0 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -64,7 +64,10 @@ static void jffs2_write_super(struct super_block *sb)
>  	if (!(sb->s_flags & MS_RDONLY)) {
>  		D1(printk(KERN_DEBUG "jffs2_write_super()\n"));
>  		jffs2_garbage_collect_trigger(c);
> -		jffs2_erase_pending_blocks(c, 0);
> +		jffs2_erase_pending_blocks(c, 3);
> +		if (!list_empty(&c->erase_complete_list) ||
> +		    !list_empty(&c->erase_pending_list))
> +			sb->s_dirt = 1;
>  		jffs2_flush_wbuf_gc(c, 0);
>  	}

I think this is not very "friendly" WRT to VFS. From VFS's POW, JFFS2
has to have it's super-block clean upon return from '->write_super()'.
By leaving the super-block dirty, you violate VFS conventions, I thing.

What I suggest you to do instead is to delegate the erasing part to the
background thread. Roughly, something like:

1. wake the JFFS2 BGT from 'jffs2_write_super()' (you'll do this by
   doing the next item).
2. Change 'jffs2_thread_should_wake()' so that it would return "true" if
   there are pending EBs to erase.
3. Erase only a couple of EBs at one loop iteration, then try to do
   other activities (GC ing)

This way you will (1) make '->write_super()'  fast.

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

Re: [PATCH] JFFS2: Erase max 3 EB's in jffs2_write_super()

[Artem Bityutskiy]

On Tue, 2010-02-02 at 10:44 +0100, Joakim Tjernlund wrote:
> Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote on 2010/02/02 10:40:00:
> >
> > Deleting big files in a JFFS2 root FS and the rebooting makes
> > the system hang until all freed EB's has been erased which
> > can take several minutes.
> > Fix the hanging by making jffs2_write_super() erase max 3 EB's
> > at a time. If there are more EB's to erase afterwards, reset the
> > s_dirty flag.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >
> >  I hope the list_empty calls doesn't need locking. If
> >  so, it can easily be added.
> >
> >  fs/jffs2/super.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> > index 9a80e8e..10393e0 100644
> > --- a/fs/jffs2/super.c
> > +++ b/fs/jffs2/super.c
> > @@ -64,7 +64,10 @@ static void jffs2_write_super(struct super_block *sb)
> >     if (!(sb->s_flags & MS_RDONLY)) {
> 
> I wonder if not MS_ACTIVE should be included in the above test too?

If this function is called with MS_ACTIVE flag, I'd say this is a bug.

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