[PATCH] jffs2: Move erasing from write_super to GC.

[Joakim Tjernlund]

David Woodhouse <dwmw2 at infradead.org> wrote on 2010/05/19 01:42:02:
>
> On Wed, 2010-05-19 at 00:44 +0200, Joakim Tjernlund wrote:
> >
> > oops, this got a lot more complicated. I will have closer look tmw.
>
> It's not so bad -- the whole thing looks like this (I'll follow it up
> with a global s/jffs2_erase_pending_trigger/jffs2_gc_thread_trigger/).
>
> It could perhaps be broken into smaller simple patches along the lines
> of:
>  - Add 'work_done' return value from jffs2_erase_pending_blocks()
>  - Call jffs2_erase_pending_blocks() from jffs2_g_c_pass()
>  - Fix the conditions under which jffs2_g_c_pass() will return -EINVAL,
>    fix the way that jffs2_reserve_space() will respond to that, and fix
>    jffs2_erase_succeeded() to actually wake up the erase_wait queue.
>  - Remove locking from jffs2_garbage_collect_trigger() and require that
>    callers have c->erase_completion_lock already held.
>  - Change most callers of jffs2_erase_pending_trigger() to call
>    jffs2_garbage_collect_trigger() instead. To comply with the locking
>    rules, this may involve moving the call up or down a few lines, or
>    sometimes adding a new lock/unlock pair around it.
>  - Remove jffs2_erase_pending_blocks() call from jffs2_write_super().
>  - Rename jffs2_erase_pending_trigger() and its only remaining caller
>    in wbuf.c to jffs2_dirty_trigger().

I have tried this on my 2.6.33 tree and it seems to work OK.
I am currenltly copying lots of files and deleting them in a loop,
so far so good :) Needed to "tweak":

>
>
> diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
> index 3ff50da..39c65ad 100644
> --- a/fs/jffs2/background.c
> +++ b/fs/jffs2/background.c
> @@ -21,12 +21,11 @@
>
>  static int jffs2_garbage_collect_thread(void *);
>
> -void jffs2_garbage_collect_trigger(struct jffs2_sb_info *c)
> +void jffs2_erase_pending_trigger(struct jffs2_sb_info *c)
>  {
> -   spin_lock(&c->erase_completion_lock);
> +   BUG_ON(spin_trylock(&c->erase_completion_lock));

This looks like a typo and I had to change that to:
BUG_ON(spin_is_locked(&c->erase_completion_lock));

>     if (c->gc_task && jffs2_thread_should_wake(c))
>        send_sig(SIGHUP, c->gc_task, 1);
> -   spin_unlock(&c->erase_completion_lock);
>  }
>
>  /* This must only ever be called when no GC thread is currently running */
> diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
> index b47679b..e8638f8 100644
> --- a/fs/jffs2/erase.c
> +++ b/fs/jffs2/erase.c
> @@ -103,9 +103,10 @@ static void jffs2_erase_block(struct jffs2_sb_info *c,
>     jffs2_erase_failed(c, jeb, bad_offset);
>  }
>
> -void jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count)
> +int jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count)
>  {
>     struct jffs2_eraseblock *jeb;
> +   int work_done = 0;
>
>     mutex_lock(&c->erase_free_sem);
>
> @@ -123,6 +124,7 @@ void jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count)
>
>           if (!--count) {
>              D1(printk(KERN_DEBUG "Count reached. jffs2_erase_pending_blocks
> leaving\n"));
> +            work_done = 1;

work_done = 1; should move outside the if, otherwise it might get set if count is big.