[PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/

[Joakim Tjernlund]

Artem Bityutskiy <dedekind1 at gmail.com> wrote on 2010/02/17 08:48:28:
> Subject: Re: [PATCH 2/2] s/jffs2_erase_pending_trigger/jffs2_garbage_collect_trigger/
>
> On Wed, 2010-02-17 at 08:35 +0100, Joakim Tjernlund wrote:
> > Artem Bityutskiy <dedekind1 at gmail.com> wrote on 2010/02/17 08:18:23:
> > >
> > > On Tue, 2010-02-16 at 15:27 +0100, Joakim Tjernlund wrote:
> > > > Artem Bityutskiy <dedekind1 at gmail.com> wrote on 2010/02/16 09:59:49:
> > > > >
> > > > > On Mon, 2010-02-15 at 17:03 +0100, Joakim Tjernlund wrote:
> > > > > > Since erasing is done in GC now, trigger GC instead.
> > > > > > jffs2_erase_pending_trigger() renamed to jffs2_dirty_trigger() and
> > > > > > used by wbuf. Remove call jffs2_garbage_collect_trigger() in
> > > > > > write_super()
> > > > > >
> > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
> > > > > > ---
> > > > > >  fs/jffs2/erase.c    |    4 +---
> > > > > >  fs/jffs2/gc.c       |    2 +-
> > > > > >  fs/jffs2/nodemgmt.c |    4 ++--
> > > > > >  fs/jffs2/os-linux.h |    2 +-
> > > > > >  fs/jffs2/scan.c     |    2 +-
> > > > > >  fs/jffs2/super.c    |    1 -
> > > > > >  fs/jffs2/wbuf.c     |    8 ++++----
> > > > > >  7 files changed, 10 insertions(+), 13 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
> > > > > > index 1ca2559..fdf9418 100644
> > > > > > --- a/fs/jffs2/erase.c
> > > > > > +++ b/fs/jffs2/erase.c
> > > > > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info
> > > > > *c, struct jffs2_eraseblo
> > > > > >     list_move_tail(&jeb->list, &c->erase_complete_list);
> > > > > >     spin_unlock(&c->erase_completion_lock);
> > > > > >     mutex_unlock(&c->erase_free_sem);
> > > > > > -   /* Ensure that kupdated calls us again to mark them clean */
> > > > > > -   jffs2_erase_pending_trigger(c);
> > > > > >  }
> > > > > >
> > > > > >  static void jffs2_erase_failed(struct jffs2_sb_info *c, struct
> > > > > jffs2_eraseblock *jeb, uint32_t bad_offset)
> > > > > > @@ -492,7 +490,7 @@ filebad:
> > > > > >
> > > > > >  refile:
> > > > > >     /* Stick it back on the list from whence it came and come back later */
> > > > > > -   jffs2_erase_pending_trigger(c);
> > > > > > +   jffs2_garbage_collect_trigger(c);
> > > > >
> > > > > But then you make the code more confusing. Indeed, readability becomes
> > > > > worse.
> > > > >
> > > > > I would just change 'jffs2_erase_pending_trigger()' and make it wake up
> > > > > the GC thread, just like 'jffs2_garbage_collect_trigger()'...
> > > >
> > > > Here we go then:
> > > >
> > > > From 96a4a9dc054f2dbd57e180e202f69c9645536e0d Mon Sep 17 00:00:00 2001
> > > > From: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
> > > > Date: Tue, 16 Feb 2010 15:18:31 +0100
> > > > Subject: [PATCH] jffs2: Make jffs2_erase_pending_trigger() initiate GC.
> > > >
> > > > Since erasing is now in the GC thread, erases should trigger
> > > > the GC task instead.
> > > > wbuf.c still wants to flush its buffer via write_super so
> > > > invent jffs2_dirty_trigger() and use that in wbuf.
> > > > Remove surplus call to jffs2_erase_pending_trigger() in erase.c
> > > > and remove jffs2_garbage_collect_trigger() from write_super as
> > > > of now write_super() should only commit dirty data to disk.
> > > >
> > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
> > > > ---
> > > >  fs/jffs2/erase.c    |    2 --
> > > >  fs/jffs2/os-linux.h |    9 +++++++--
> > > >  fs/jffs2/super.c    |    1 -
> > > >  fs/jffs2/wbuf.c     |    2 +-
> > > >  4 files changed, 8 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
> > > > index 1ca2559..5616658 100644
> > > > --- a/fs/jffs2/erase.c
> > > > +++ b/fs/jffs2/erase.c
> > > > @@ -172,8 +172,6 @@ static void jffs2_erase_succeeded(struct jffs2_sb_info
> > > *c, struct jffs2_eraseblo
> > > >     list_move_tail(&jeb->list, &c->erase_complete_list);
> > > >     spin_unlock(&c->erase_completion_lock);
> > > >     mutex_unlock(&c->erase_free_sem);
> > > > -   /* Ensure that kupdated calls us again to mark them clean */
> > > > -   jffs2_erase_pending_trigger(c);
> > > >  }
> > >
> > > Looks like BGT should be triggered from here in order to write the clean
> > > marker.
> >
> > How so? JFFS2 is already running jffs2_erase_pending_blocks() and
> > has just completed an erase, the next thing it will do is to mark it with a clean
> > marker. To me it looks like belts and suspenders: kick it again just in casewe missed
> > something.
>
> OK. I expect you will send the final version of your 2 patches, then
> I'll put them to my l2 tree, right?

OK, will do shortly. What is your l2 tree? I figured you would
push this to the mtd tree.

 Jocke