Re: [PATCH] [JFFS2] Fix free space leaking

[David Woodhouse <dwmw2@infradead.org>]

On Thu, 2008-04-03 at 11:03 +0200, Martin Creutziger wrote:
> On Thu, 2008-02-21 at 20:39 +0900, an unknown sender wrote:
> > On Thu, 2008-02-21 at 12:11 +0100, J?rn Engel wrote:
> > > No idea, but it might help to add dwmw2 to Cc:.
> > 
> > dwmw2 moderately confused by it. I don't see why it's correct to account
> > for the space as 'free' when it isn't really. If it's causing a
> > persistent leak, shouldn't we deal with that when the block is erased
> > instead?
> 
> Hi, 
> 
> I am currently testing the patch on 2.6.24.2 (as a result of
> http://lists.infradead.org/pipermail/linux-mtd/2008-March/021024.html),
> and so far it seems to at least cure the symptoms, as in: The partition
> does not "fill up" any more so far. 
> But: For later production use I would certainly prefer a patch that also
> dwmw2 agrees being a cure to the _cause_, not just the symptoms.
> Any suggestions?

I have reproduced this locally with some extra 'paranoia' debug checks,
and I understand it now. The original patch was doing fairly much the
right thing, but ideally would have looked more like this:

--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
@@ -459,8 +459,9 @@ static void jffs2_mark_erased_block(struct jffs2_sb_info *c,
        down(&c->erase_free_sem);
        spin_lock(&c->erase_completion_lock);
        c->erasing_size -= c->sector_size;
-       c->free_size += jeb->free_size;
-       c->used_size += jeb->used_size;
+       /* We already accounted for the cleanmarker in c->{used,free}_size,
+          so just add the eraseblock size now. */
+       c->free_size += c->sector_size;
 
        jffs2_dbg_acct_sanity_check_nolock(c,jeb);
        jffs2_dbg_acct_paranoia_check_nolock(c, jeb);


In fact I've cleaned it up a little more to fix the fact that we were
calling jffs2_link_node_ref() without the required locking:
http://git.infradead.org/mtd-2.6.git?a=commitdiff;h=014b164e1392a166fe96e003d2f0e7ad2e2a0bb7

Thanks for your patience, and for showing me exactly where to look.

-- 
dwmw2