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

[Joakim Tjernlund]

>
> David Woodhouse <dwmw2 at infradead.org> wrote on 2010/05/14 12:35:04:
> >
> > On Fri, 2010-05-14 at 12:10 +0200, Joakim Tjernlund wrote:
> > >  The only callers of jffs2_erase_pending_blocks() now call it with a
> > > > 'count' argument of 1. So perhaps it's now misnamed and the 's' and the
> > > > extra argument should be dropped?
> > >
> > > I didn't want to change to much and who knows, maybe someone wants
> > > to erase more than one block in the future. Removing the
> > > count could be an add on patch once this patch has proven itself.
> >
> > Yeah, that makes some sense.
> >
> > > > I don't much like the calculation you've added to the end of that
> > > > function either, which really ought to be under locks (even though right
> > > > now I suspect it doesn't hurt). Why recalculate that at all, though --
> > >
> > > Why does a simple list test need locks?
> >
> > Because it's not just about the test itself. It's also about the memory
> > barriers. Some other CPU could have changed the list (under locks) but
> > unless you have the memory barrier which is implicit in the spinlock,
> > you might see old data.
>
> old data doesn't matter here I think.
>
> >
> > > > why not keep a 'ret' variable which defaults to 0 but is set to 1 just
> > > > before the 'goto done' which is the only way out of the function where
> > > > the return value should be non-zero anyway?
> > >
> > > That would not be the same, would it? One wants to know if the lists
> > > are empty AFTER erasing count blocks.
> >
> > Hm, does one? There's precisely one place we use this return value, in
> > the GC thread. Can you explain the logic of what you were doing there?
>
> Sure, return 1 if there are more blocks left in the list after
> erasing count. That way the caller knows if there are any block left
> to erase.
>
> > It looks like you really wanted it to return a flag saying whether it
> > actually _did_ anything or not. And if it did, that's your work for this
> > GC wakeup and you don't call jffs2_garbage_collect_pass(). Why are you
> > returning a value which tells whether there's more work to do?
>
> hmm, I guess the simpler method like you suggested would work too.
> Details are a bit fuzzy now.
>
> >
> > >  I guess I could move the list empty
> > > check before goto done, but that would not really change anything.
> >
> > Ah, yes. Instead of setting ret=1 at the 'goto done', you'd actually do
> > the test somewhere there too, before dropping the locks. Assuming that
> > this really is the return value you need to return, rather than a simple
> > 'work_done' flag.

How about this then? I have changed jffs2_erase_pending_blocks() to use
the simpler work_done flag: