public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH][JFFS2] Fix garbage collector block search
@ 2007-12-12 15:51 Alexander Belyakov
  2008-01-14 10:23 ` Alexander Belyakov
  2008-01-14 13:28 ` Jörn Engel
  0 siblings, 2 replies; 25+ messages in thread
From: Alexander Belyakov @ 2007-12-12 15:51 UTC (permalink / raw)
  To: David Woodhouse, linux-mtd@lists.infradead.org,
	Alexander Belyakov

Hello,

The patch checks if erasable_list is empty in jffs2_refile_wbuf_blocks() moving at least one block to that list. So jffs2_find_gc_block() will not fail sometimes with "jffs2: No clean, dirty _or_ erasable blocks to GC from! Where are they all?" message.

Signed-off-by: Alexander Belyakov <alexander.belyakov@intel.com>

diff -uNr a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
--- a/fs/jffs2/wbuf.c	2007-12-03 17:00:16.000000000 +0300
+++ b/fs/jffs2/wbuf.c	2007-12-11 19:03:03.000000000 +0300
@@ -115,7 +115,7 @@
 
 		D1(printk(KERN_DEBUG "Removing eraseblock at 0x%08x from erasable_pending_wbuf_list...\n", jeb->offset));
 		list_del(this);
-		if ((jiffies + (n++)) & 127) {
+		if (((jiffies + (n++)) & 127) && !list_empty(&c->erasable_list)) {
 			/* Most of the time, we just erase it immediately. Otherwise we
 			   spend ages scanning it on mount, etc. */
 			D1(printk(KERN_DEBUG "...and adding to erase_pending_list\n"));

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2007-12-12 15:51 [PATCH][JFFS2] Fix garbage collector block search Alexander Belyakov
@ 2008-01-14 10:23 ` Alexander Belyakov
  2008-01-14 13:28 ` Jörn Engel
  1 sibling, 0 replies; 25+ messages in thread
From: Alexander Belyakov @ 2008-01-14 10:23 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Alexander Belyakov, linux-mtd@lists. infradead. org

On Dec 12, 2007 6:51 PM, Alexander Belyakov <abelyako@mail.ru> wrote:
> Hello,
>
> The patch checks if erasable_list is empty in jffs2_refile_wbuf_blocks() moving at least one block to that list. So jffs2_find_gc_block() will not fail sometimes with "jffs2: No clean, dirty _or_ erasable blocks to GC from! Where are they all?" message.
>
> Signed-off-by: Alexander Belyakov <alexander.belyakov@intel.com>
>
> diff -uNr a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
> --- a/fs/jffs2/wbuf.c   2007-12-03 17:00:16.000000000 +0300
> +++ b/fs/jffs2/wbuf.c   2007-12-11 19:03:03.000000000 +0300
> @@ -115,7 +115,7 @@
>
>                 D1(printk(KERN_DEBUG "Removing eraseblock at 0x%08x from erasable_pending_wbuf_list...\n", jeb->offset));
>                 list_del(this);
> -               if ((jiffies + (n++)) & 127) {
> +               if (((jiffies + (n++)) & 127) && !list_empty(&c->erasable_list)) {
>                         /* Most of the time, we just erase it immediately. Otherwise we
>                            spend ages scanning it on mount, etc. */
>                         D1(printk(KERN_DEBUG "...and adding to erase_pending_list\n"));
>
>

Hello, David.

I've sent this jffs2 fix about month ago. Have you received applicable
patch? It should be applicable -- I've sent it to myself and checked
it before sending out.

Have you any comments on this patch?

Thanks,
Alexander

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2007-12-12 15:51 [PATCH][JFFS2] Fix garbage collector block search Alexander Belyakov
  2008-01-14 10:23 ` Alexander Belyakov
@ 2008-01-14 13:28 ` Jörn Engel
  2008-01-15 12:24   ` Alexander Belyakov
  1 sibling, 1 reply; 25+ messages in thread
From: Jörn Engel @ 2008-01-14 13:28 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: linux-mtd@lists.infradead.org, David Woodhouse,
	Alexander Belyakov

On Wed, 12 December 2007 18:51:28 +0300, Alexander Belyakov wrote:
> 
> The patch checks if erasable_list is empty in jffs2_refile_wbuf_blocks() moving at least one block to that list. So jffs2_find_gc_block() will not fail sometimes with "jffs2: No clean, dirty _or_ erasable blocks to GC from! Where are they all?" message.

If the erasable_list is empty, we push it to the list for someone else
to erase it later instead of doing it now?  Nonsense.

If it makes a difference whether the block is erasable or erase_pending,
we have a bug.  And that bug wants a fix, not a paper bandaid.

> Signed-off-by: Alexander Belyakov <alexander.belyakov@intel.com>
> 
> diff -uNr a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
> --- a/fs/jffs2/wbuf.c	2007-12-03 17:00:16.000000000 +0300
> +++ b/fs/jffs2/wbuf.c	2007-12-11 19:03:03.000000000 +0300
> @@ -115,7 +115,7 @@
>  
>  		D1(printk(KERN_DEBUG "Removing eraseblock at 0x%08x from erasable_pending_wbuf_list...\n", jeb->offset));
>  		list_del(this);
> -		if ((jiffies + (n++)) & 127) {
> +		if (((jiffies + (n++)) & 127) && !list_empty(&c->erasable_list)) {
>  			/* Most of the time, we just erase it immediately. Otherwise we
>  			   spend ages scanning it on mount, etc. */
>  			D1(printk(KERN_DEBUG "...and adding to erase_pending_list\n"));
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Jörn

-- 
Joern's library part 14:
http://www.sandpile.org/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-01-14 13:28 ` Jörn Engel
@ 2008-01-15 12:24   ` Alexander Belyakov
  2008-01-15 13:33     ` Jörn Engel
  0 siblings, 1 reply; 25+ messages in thread
From: Alexander Belyakov @ 2008-01-15 12:24 UTC (permalink / raw)
  To: Jörn Engel, David Woodhouse, linux-mtd@lists.infradead.org
  Cc: Alexander Belyakov

On Jan 14, 2008 4:28 PM, Jörn Engel <joern@logfs.org> wrote:
>
> If it makes a difference whether the block is erasable or erase_pending,
> we have a bug.  And that bug wants a fix, not a paper bandaid.
>

I've posted this bug details almost half year ago.
http://lists.infradead.org/pipermail/linux-mtd/2007-June/018756.html

I do agree with your objection. If you have perfect patch ideas please
let me know.

I'll think about another patch as well.

Thanks,
Alexander

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-01-15 12:24   ` Alexander Belyakov
@ 2008-01-15 13:33     ` Jörn Engel
  2008-01-18 16:28       ` Alexander Belyakov
  0 siblings, 1 reply; 25+ messages in thread
From: Jörn Engel @ 2008-01-15 13:33 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: linux-mtd@lists.infradead.org, Jörn Engel, David Woodhouse,
	Alexander Belyakov

On Tue, 15 January 2008 15:24:03 +0300, Alexander Belyakov wrote:
> On Jan 14, 2008 4:28 PM, Jörn Engel <joern@logfs.org> wrote:
> >
> > If it makes a difference whether the block is erasable or erase_pending,
> > we have a bug.  And that bug wants a fix, not a paper bandaid.
> >
> 
> I've posted this bug details almost half year ago.
> http://lists.infradead.org/pipermail/linux-mtd/2007-June/018756.html
> 
> I do agree with your objection. If you have perfect patch ideas please
> let me know.

Right now the important thing is to dig deeper and understand the nature
of this bug.  You can reproduce it, that is good.  We also know that it
makes a difference whether the block is on one list or the other.  But
we don't know yet, what difference exactly it makes.  That should be the
next step.

Jörn

-- 
tglx1 thinks that joern should get a (TM) for "Thinking Is Hard"
-- Thomas Gleixner

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-01-15 13:33     ` Jörn Engel
@ 2008-01-18 16:28       ` Alexander Belyakov
  2008-02-25 12:50         ` Jörn Engel
  2008-04-22 16:47         ` David Woodhouse
  0 siblings, 2 replies; 25+ messages in thread
From: Alexander Belyakov @ 2008-01-18 16:28 UTC (permalink / raw)
  To: Jörn Engel
  Cc: linux-mtd@lists.infradead.org, David Woodhouse,
	Alexander Belyakov

On 1/15/08, Jörn Engel <joern@logfs.org> wrote:
>
> Right now the important thing is to dig deeper and understand the nature
> of this bug.  You can reproduce it, that is good.  We also know that it
> makes a difference whether the block is on one list or the other.  But
> we don't know yet, what difference exactly it makes.

Question. What is success criteria for jffs2_garbage_collect_pass() execution?

Why asking? In the case described above jffs2_find_gc_block() fails to
find erase block for garbage collection but falling into
jffs2_flush_wbuf_pad(c) which produces amount of erasing blocks. So
jffs2_garbage_collect_pass() sees no single block for garbage
collection, but filesystem still recieves fresh erasing blocks upon
execution.

Is it success or failure? Theoretically.

Thanks,
Alexander

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-01-18 16:28       ` Alexander Belyakov
@ 2008-02-25 12:50         ` Jörn Engel
  2008-04-22 16:47         ` David Woodhouse
  1 sibling, 0 replies; 25+ messages in thread
From: Jörn Engel @ 2008-02-25 12:50 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: David Woodhouse, Jörn Engel, linux-mtd@lists.infradead.org,
	Alexander Belyakov

[ I have already written a reply, but somehow that got lost.  Sorry
about the delay. ]

On Fri, 18 January 2008 19:28:08 +0300, Alexander Belyakov wrote:
> On 1/15/08, Jörn Engel <joern@logfs.org> wrote:
> >
> > Right now the important thing is to dig deeper and understand the nature
> > of this bug.  You can reproduce it, that is good.  We also know that it
> > makes a difference whether the block is on one list or the other.  But
> > we don't know yet, what difference exactly it makes.
> 
> Question. What is success criteria for jffs2_garbage_collect_pass() execution?
> 
> Why asking? In the case described above jffs2_find_gc_block() fails to
> find erase block for garbage collection but falling into
> jffs2_flush_wbuf_pad(c) which produces amount of erasing blocks. So
> jffs2_garbage_collect_pass() sees no single block for garbage
> collection, but filesystem still recieves fresh erasing blocks upon
> execution.
> 
> Is it success or failure? Theoretically.

Maybe the best answer to this is in jffs2_do_reserve_space:
        /* this needs a little more thought (true <tglx> :)) */

The exit criterium of jffs2_do_reserve_space() should be one of a)
enough free space was found or b) there is not enough free space.  If
the function returns with b) while there actually is more space to be
freed by calling jffs2_flush_wbuf_pad(), that is a bug.

Is this the case?  If it is, there should be blocks on the
erasable_pending_wbuf_list.  And that case is handled (correctly at
first glance) in jffs2_find_nextblock.

That is about as much digging as I will do.  My personal interest is
more in replacing JFFS2 than hunting arcane bugs in it. ;)

Jörn

-- 
And spam is a useful source of entropy for /dev/random too!
-- Jasmine Strong

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-01-18 16:28       ` Alexander Belyakov
  2008-02-25 12:50         ` Jörn Engel
@ 2008-04-22 16:47         ` David Woodhouse
  2008-04-23 11:31           ` Alexander Belyakov
  1 sibling, 1 reply; 25+ messages in thread
From: David Woodhouse @ 2008-04-22 16:47 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org

On Fri, 2008-01-18 at 19:28 +0300, Alexander Belyakov wrote:
> On 1/15/08, Jörn Engel <joern@logfs.org> wrote:
> >
> > Right now the important thing is to dig deeper and understand the nature
> > of this bug.  You can reproduce it, that is good.  We also know that it
> > makes a difference whether the block is on one list or the other.  But
> > we don't know yet, what difference exactly it makes.
> 
> Question. What is success criteria for jffs2_garbage_collect_pass() execution?
> 
> Why asking? In the case described above jffs2_find_gc_block() fails to
> find erase block for garbage collection but falling into
> jffs2_flush_wbuf_pad(c) which produces amount of erasing blocks. So
> jffs2_garbage_collect_pass() sees no single block for garbage
> collection, but filesystem still recieves fresh erasing blocks upon
> execution.
> 
> Is it success or failure? Theoretically.

It depends. There are two callers of jffs2_garbage_collect_pass().

For jffs2_flush_wbuf_gc() it's not progress, so it should be handled
like failure.

For jffs2_reserve_space(), we can treat it as success... or we can have
a special case which calls jffs2_erase_pending_blocks() right away to
wait for the block to erase.

How about this... does it solve the problem for you?

diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
index 26c7992..f2c6e88 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -221,6 +221,12 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c)
 		jeb = jffs2_find_gc_block(c);
 
 	if (!jeb) {
+		/* Couldn't find a free block. But maybe we can just erase one and make 'progress'? */
+		if (!list_empty(&c->erase_pending_list)) {
+			spin_unlock(&c->erase_completion_lock);
+			mutex_unlock(&c->alloc_sem);
+			return -EAGAIN;
+		}
 		D1 (printk(KERN_NOTICE "jffs2: Couldn't find erase block to garbage collect!\n"));
 		spin_unlock(&c->erase_completion_lock);
 		mutex_unlock(&c->alloc_sem);
diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c
index 747a73f..952a828 100644
--- a/fs/jffs2/nodemgmt.c
+++ b/fs/jffs2/nodemgmt.c
@@ -116,7 +116,12 @@ int jffs2_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,
 			spin_unlock(&c->erase_completion_lock);
 
 			ret = jffs2_garbage_collect_pass(c);
-			if (ret)
+			if (ret == -EAGAIN) {
+				jffs2_erase_pending_blocks(c, 1);
+				goto again;
+			}
+
+			if (ret) 
 				return ret;
 
 			cond_resched();
@@ -124,6 +129,7 @@ int jffs2_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,
 			if (signal_pending(current))
 				return -EINTR;
 
+		again:
 			mutex_lock(&c->alloc_sem);
 			spin_lock(&c->erase_completion_lock);
 		}


-- 
dwmw2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-22 16:47         ` David Woodhouse
@ 2008-04-23 11:31           ` Alexander Belyakov
  2008-04-23 11:32             ` David Woodhouse
  0 siblings, 1 reply; 25+ messages in thread
From: Alexander Belyakov @ 2008-04-23 11:31 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org

On Tue, Apr 22, 2008 at 8:47 PM, David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Fri, 2008-01-18 at 19:28 +0300, Alexander Belyakov wrote:
>  >
>  > Question. What is success criteria for jffs2_garbage_collect_pass() execution?
>  >
>
>  It depends. There are two callers of jffs2_garbage_collect_pass().
>
>  For jffs2_flush_wbuf_gc() it's not progress, so it should be handled
>  like failure.
>
>  For jffs2_reserve_space(), we can treat it as success... or we can have
>  a special case which calls jffs2_erase_pending_blocks() right away to
>  wait for the block to erase.
>
>  How about this... does it solve the problem for you?
>

I like this special case for jffs2_reserve_space(). It solves the problem.

Thank you,
Alexander

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-23 11:31           ` Alexander Belyakov
@ 2008-04-23 11:32             ` David Woodhouse
  2008-04-23 13:08               ` Alexander Belyakov
  0 siblings, 1 reply; 25+ messages in thread
From: David Woodhouse @ 2008-04-23 11:32 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org

On Wed, 2008-04-23 at 15:31 +0400, Alexander Belyakov wrote:
> On Tue, Apr 22, 2008 at 8:47 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > On Fri, 2008-01-18 at 19:28 +0300, Alexander Belyakov wrote:
> >  >
> >  > Question. What is success criteria for jffs2_garbage_collect_pass() execution?
> >  >
> >
> >  It depends. There are two callers of jffs2_garbage_collect_pass().
> >
> >  For jffs2_flush_wbuf_gc() it's not progress, so it should be handled
> >  like failure.
> >
> >  For jffs2_reserve_space(), we can treat it as success... or we can have
> >  a special case which calls jffs2_erase_pending_blocks() right away to
> >  wait for the block to erase.
> >
> >  How about this... does it solve the problem for you?
> >
> 
> I like this special case for jffs2_reserve_space(). It solves the problem.

I'm playing with it now, with the intent of committing it. Can you show
me how to reproduce the problem?

-- 
dwmw2

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-23 11:32             ` David Woodhouse
@ 2008-04-23 13:08               ` Alexander Belyakov
  2008-04-23 14:01                 ` David Woodhouse
  0 siblings, 1 reply; 25+ messages in thread
From: Alexander Belyakov @ 2008-04-23 13:08 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org

On Wed, Apr 23, 2008 at 3:32 PM, David Woodhouse <dwmw2@infradead.org> wrote:
>
>  I'm playing with it now, with the intent of committing it. Can you show
>  me how to reproduce the problem?
>

The most easy way to reproduce it is to fill jffs2 volume with single
large file until there is no empty space, then remove this file and
immediately try to write another file.

Something like:
---
#!/bin/sh
rm -rf /mnt/vol1/*
dd if=/dev/urandom of=/mnt/vol1/test.file bs=4096 count=32768
rm /mnt/vol1/test.file
dd if=/dev/urandom of=/mnt/vol1/test_new.file bs=2048 count=1024
-------

The bug is better reproduced on small volumes (for me it is just
4Mbytes). The first dd should exit with "No space left on device"
error to be sure no free space left. The last dd should always
succeed. If it fails - you catch the bug. Sometimes it is necessary to
run this script in several time in a row before last dd exits with an
error.

Thanks,
Alexander

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-23 13:08               ` Alexander Belyakov
@ 2008-04-23 14:01                 ` David Woodhouse
  2008-04-23 14:30                   ` David Woodhouse
  0 siblings, 1 reply; 25+ messages in thread
From: David Woodhouse @ 2008-04-23 14:01 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org

On Wed, 2008-04-23 at 17:08 +0400, Alexander Belyakov wrote:
> On Wed, Apr 23, 2008 at 3:32 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> >
> >  I'm playing with it now, with the intent of committing it. Can you show
> >  me how to reproduce the problem?
> >
> 
> The most easy way to reproduce it is to fill jffs2 volume with single
> large file until there is no empty space, then remove this file and
> immediately try to write another file.
> 
> Something like:
> ---
> #!/bin/sh
> rm -rf /mnt/vol1/*
> dd if=/dev/urandom of=/mnt/vol1/test.file bs=4096 count=32768
> rm /mnt/vol1/test.file
> dd if=/dev/urandom of=/mnt/vol1/test_new.file bs=2048 count=1024
> -------
> 
> The bug is better reproduced on small volumes (for me it is just
> 4Mbytes). The first dd should exit with "No space left on device"
> error to be sure no free space left. The last dd should always
> succeed. If it fails - you catch the bug. Sometimes it is necessary to
> run this script in several time in a row before last dd exits with an
> error.

Hm, I can't make it fail. I'm using nandsim, and have enabled the
'Museum IDs' so that I can have a 4MiB device (I've also tried 2MiB).

I never even see the 'Synching wbuf in order to reuse
erasable_pending_wbuf_list blocks' message, whether I have debugging
enabled or whether I just unable that one message.

-- 
dwmw2

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-23 14:01                 ` David Woodhouse
@ 2008-04-23 14:30                   ` David Woodhouse
  2008-04-23 15:25                     ` Alexander Belyakov
  0 siblings, 1 reply; 25+ messages in thread
From: David Woodhouse @ 2008-04-23 14:30 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Wed, 2008-04-23 at 15:01 +0100, David Woodhouse wrote:
> Hm, I can't make it fail. I'm using nandsim, and have enabled the
> 'Museum IDs' so that I can have a 4MiB device (I've also tried 2MiB).

Hm. I had to disable XATTR support and _now_ I can make it fail. I can
watch my patch do the right thing, so I'll commit it. Thanks for the
diagnosis.

Not sure why it wasn't failing with XATTR enabled. At first glance I
think it was because all the blocks were going on the very_dirty_list
instead of the erasable_pending_wbuf_list, when I deleted the file.
I hope we don't have a space leak with XATTRs.
 
-- 
dwmw2

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-23 14:30                   ` David Woodhouse
@ 2008-04-23 15:25                     ` Alexander Belyakov
  2008-04-23 15:30                       ` David Woodhouse
  0 siblings, 1 reply; 25+ messages in thread
From: Alexander Belyakov @ 2008-04-23 15:25 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Wed, Apr 23, 2008 at 6:30 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2008-04-23 at 15:01 +0100, David Woodhouse wrote:
>  > Hm, I can't make it fail. I'm using nandsim, and have enabled the
>  > 'Museum IDs' so that I can have a 4MiB device (I've also tried 2MiB).
>
>  Hm. I had to disable XATTR support and _now_ I can make it fail. I can
>  watch my patch do the right thing, so I'll commit it. Thanks for the
>  diagnosis.

Good. You're welcome.

>
>  Not sure why it wasn't failing with XATTR enabled. At first glance I
>  think it was because all the blocks were going on the very_dirty_list
>  instead of the erasable_pending_wbuf_list, when I deleted the file.
>  I hope we don't have a space leak with XATTRs.
>

Continuously running script from my previous message, I've just
noticed that space leak does exist. Available space (reported by df)
decreases with each iteration. I've not investigated this in details
yet, but it seems the behavior is the same for XATTR support enabled
and disabled.

Thanks,
Alexander

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-23 15:25                     ` Alexander Belyakov
@ 2008-04-23 15:30                       ` David Woodhouse
  2008-04-24  7:17                         ` Alexander Belyakov
  0 siblings, 1 reply; 25+ messages in thread
From: David Woodhouse @ 2008-04-23 15:30 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Wed, 2008-04-23 at 19:25 +0400, Alexander Belyakov wrote:
> >  Not sure why it wasn't failing with XATTR enabled. At first glance I
> >  think it was because all the blocks were going on the very_dirty_list
> >  instead of the erasable_pending_wbuf_list, when I deleted the file.
> >  I hope we don't have a space leak with XATTRs.
> >
> 
> Continuously running script from my previous message, I've just
> noticed that space leak does exist. Available space (reported by df)
> decreases with each iteration. I've not investigated this in details
> yet, but it seems the behavior is the same for XATTR support enabled
> and disabled.

I wouldn't worry too much about what jffs2_statfs() says. We really do
make crap up there :)

-- 
dwmw2

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-23 15:30                       ` David Woodhouse
@ 2008-04-24  7:17                         ` Alexander Belyakov
  2008-04-24  7:28                           ` David Woodhouse
  0 siblings, 1 reply; 25+ messages in thread
From: Alexander Belyakov @ 2008-04-24  7:17 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Wed, Apr 23, 2008 at 7:30 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Wed, 2008-04-23 at 19:25 +0400, Alexander Belyakov wrote:
>  >
>  > Continuously running script from my previous message, I've just
>  > noticed that space leak does exist. Available space (reported by df)
>  > decreases with each iteration. I've not investigated this in details
>  > yet, but it seems the behavior is the same for XATTR support enabled
>  > and disabled.
>
>  I wouldn't worry too much about what jffs2_statfs() says. We really do
>  make crap up there :)
>

Hm... This script was just the easiest way to reproduce gc bug, but
not the only. Are you sure this leak won't appear in other sane cases?

-a

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24  7:17                         ` Alexander Belyakov
@ 2008-04-24  7:28                           ` David Woodhouse
  2008-04-24  7:48                             ` Alexander Belyakov
  0 siblings, 1 reply; 25+ messages in thread
From: David Woodhouse @ 2008-04-24  7:28 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, 2008-04-24 at 11:17 +0400, Alexander Belyakov wrote:
> Hm... This script was just the easiest way to reproduce gc bug, but
> not the only. Are you sure this leak won't appear in other sane cases?

I don't believe there's really a leak; certainly not without XATTR.
My own original suspicion was based on a quick glance at debug output
and not seeing blocks getting refiled onto the erase_pending_wbuf list.
But grep shows my initial impression there was wrong.

If the only other evidence of a leak is that jffs2_statfs() reports
fluctuating values, then I'm no more likely to take it seriously than if
you tell me "a pixie told me" :)

Really, we make shit up in jffs2_statfs().

Or do you have other reasons to believe there's a leak? Other than the
one I fixed in commit 014b164e, that is. Although that's only on NOR.

-- 
dwmw2

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24  7:28                           ` David Woodhouse
@ 2008-04-24  7:48                             ` Alexander Belyakov
  2008-04-24  7:51                               ` David Woodhouse
  0 siblings, 1 reply; 25+ messages in thread
From: Alexander Belyakov @ 2008-04-24  7:48 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, Apr 24, 2008 at 11:28 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2008-04-24 at 11:17 +0400, Alexander Belyakov wrote:
>  > Hm... This script was just the easiest way to reproduce gc bug, but
>  > not the only. Are you sure this leak won't appear in other sane cases?
>
>  If the only other evidence of a leak is that jffs2_statfs() reports
>  fluctuating values, then I'm no more likely to take it seriously than if
>  you tell me "a pixie told me" :)
>

Values are not fluctuating. After about 40 iterations I have no space
to write 2Mbytes file on 4MiB volume constantly getting "No space left
on device" until remount.

It might be not a real leak, but anyway pixie doesn't feel this is a
correct behavior. :)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24  7:48                             ` Alexander Belyakov
@ 2008-04-24  7:51                               ` David Woodhouse
  2008-04-24 10:14                                 ` Alexander Belyakov
  0 siblings, 1 reply; 25+ messages in thread
From: David Woodhouse @ 2008-04-24  7:51 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, 2008-04-24 at 11:48 +0400, Alexander Belyakov wrote:
> Values are not fluctuating. After about 40 iterations I have no space
> to write 2Mbytes file on 4MiB volume constantly getting "No space left
> on device" until remount.

With no existing files?

> It might be not a real leak, but anyway pixie doesn't feel this is a
> correct behavior. :)

Ah, OK -- that's definitely not what I was seeing here. I'll try harder
to reproduce.

Does the space come back when you remount? And are you running with the
new paranoia check I added in commit 85a62db6?

-- 
dwmw2

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24  7:51                               ` David Woodhouse
@ 2008-04-24 10:14                                 ` Alexander Belyakov
  2008-04-24 10:32                                   ` David Woodhouse
  0 siblings, 1 reply; 25+ messages in thread
From: Alexander Belyakov @ 2008-04-24 10:14 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, Apr 24, 2008 at 11:51 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2008-04-24 at 11:48 +0400, Alexander Belyakov wrote:
>  > Values are not fluctuating. After about 40 iterations I have no space
>  > to write 2Mbytes file on 4MiB volume constantly getting "No space left
>  > on device" until remount.
>
>  With no existing files?

No files.

>
>
>  > It might be not a real leak, but anyway pixie doesn't feel this is a
>  > correct behavior. :)
>
>  Ah, OK -- that's definitely not what I was seeing here. I'll try harder
>  to reproduce.
>
>  Does the space come back when you remount? And are you running with the
>  new paranoia check I added in commit 85a62db6?
>

Space comes back after remount.

I'm able to reproduce problem with stable 2.6.25 kernel, but I see no
such fails with the latest mtd-2.6.git snapshot. It seems changes in
jffs2 since 2.6.25 merge address this issue somehow.

If I hit something similar once again I would let you know.

-a

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24 10:14                                 ` Alexander Belyakov
@ 2008-04-24 10:32                                   ` David Woodhouse
  2008-04-24 10:42                                     ` Alexander Belyakov
  0 siblings, 1 reply; 25+ messages in thread
From: David Woodhouse @ 2008-04-24 10:32 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, 2008-04-24 at 14:14 +0400, Alexander Belyakov wrote:
> 
> Space comes back after remount.
> 
> I'm able to reproduce problem with stable 2.6.25 kernel, but I see no
> such fails with the latest mtd-2.6.git snapshot. It seems changes in
> jffs2 since 2.6.25 merge address this issue somehow.

Hm, interesting. I don't see any change which should have caused that.
The cleanmarker thing shouldn't make any difference on NAND. And finally
dropping TEST_TOTLEN shouldn't have made any difference either.

-- 
dwmw2

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24 10:32                                   ` David Woodhouse
@ 2008-04-24 10:42                                     ` Alexander Belyakov
  2008-04-24 11:05                                       ` David Woodhouse
  0 siblings, 1 reply; 25+ messages in thread
From: Alexander Belyakov @ 2008-04-24 10:42 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, Apr 24, 2008 at 2:32 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2008-04-24 at 14:14 +0400, Alexander Belyakov wrote:
>  >
>  > Space comes back after remount.
>  >
>  > I'm able to reproduce problem with stable 2.6.25 kernel, but I see no
>  > such fails with the latest mtd-2.6.git snapshot. It seems changes in
>  > jffs2 since 2.6.25 merge address this issue somehow.
>
>  Hm, interesting. I don't see any change which should have caused that.
>  The cleanmarker thing shouldn't make any difference on NAND. And finally
>  dropping TEST_TOTLEN shouldn't have made any difference either.
>

I was checking this on Sibley-like parts, not NAND. Sorry for confusion.

-a

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24 10:42                                     ` Alexander Belyakov
@ 2008-04-24 11:05                                       ` David Woodhouse
  2008-04-24 11:11                                         ` Jörn Engel
  2008-04-24 11:21                                         ` Alexander Belyakov
  0 siblings, 2 replies; 25+ messages in thread
From: David Woodhouse @ 2008-04-24 11:05 UTC (permalink / raw)
  To: Alexander Belyakov
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, 2008-04-24 at 14:42 +0400, Alexander Belyakov wrote:
> I was checking this on Sibley-like parts, not NAND. Sorry for
> confusion.

Oh, so you have an inline cleanmarker? That makes sense. You'll lose
c->cleanmarker_size from c->free_size for every block you erase.

-- 
dwmw2

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24 11:05                                       ` David Woodhouse
@ 2008-04-24 11:11                                         ` Jörn Engel
  2008-04-24 11:21                                         ` Alexander Belyakov
  1 sibling, 0 replies; 25+ messages in thread
From: Jörn Engel @ 2008-04-24 11:11 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, 24 April 2008 12:05:03 +0100, David Woodhouse wrote:
> On Thu, 2008-04-24 at 14:42 +0400, Alexander Belyakov wrote:
> > I was checking this on Sibley-like parts, not NAND. Sorry for
> > confusion.
> 
> Oh, so you have an inline cleanmarker? That makes sense. You'll lose
> c->cleanmarker_size from c->free_size for every block you erase.

And I believe that is rather large on sibley - 1024 bytes or so.

Jörn

-- 
A quarrel is quickly settled when deserted by one party; there is
no battle unless there be two.
-- Seneca

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH][JFFS2] Fix garbage collector block search
  2008-04-24 11:05                                       ` David Woodhouse
  2008-04-24 11:11                                         ` Jörn Engel
@ 2008-04-24 11:21                                         ` Alexander Belyakov
  1 sibling, 0 replies; 25+ messages in thread
From: Alexander Belyakov @ 2008-04-24 11:21 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Alexander Belyakov, Jörn Engel,
	linux-mtd@lists.infradead.org, KaiGai Kohei

On Thu, Apr 24, 2008 at 3:05 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2008-04-24 at 14:42 +0400, Alexander Belyakov wrote:
>  > I was checking this on Sibley-like parts, not NAND. Sorry for
>  > confusion.
>
>  Oh, so you have an inline cleanmarker? That makes sense. You'll lose
>  c->cleanmarker_size from c->free_size for every block you erase.
>

So you are speaking about "Fix free space leak with in-band
cleanmarkers" patch. Right? It seems I've overlooked it. Now I see
what was happening there - thank you.

Alexander

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2008-04-24 11:21 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-12 15:51 [PATCH][JFFS2] Fix garbage collector block search Alexander Belyakov
2008-01-14 10:23 ` Alexander Belyakov
2008-01-14 13:28 ` Jörn Engel
2008-01-15 12:24   ` Alexander Belyakov
2008-01-15 13:33     ` Jörn Engel
2008-01-18 16:28       ` Alexander Belyakov
2008-02-25 12:50         ` Jörn Engel
2008-04-22 16:47         ` David Woodhouse
2008-04-23 11:31           ` Alexander Belyakov
2008-04-23 11:32             ` David Woodhouse
2008-04-23 13:08               ` Alexander Belyakov
2008-04-23 14:01                 ` David Woodhouse
2008-04-23 14:30                   ` David Woodhouse
2008-04-23 15:25                     ` Alexander Belyakov
2008-04-23 15:30                       ` David Woodhouse
2008-04-24  7:17                         ` Alexander Belyakov
2008-04-24  7:28                           ` David Woodhouse
2008-04-24  7:48                             ` Alexander Belyakov
2008-04-24  7:51                               ` David Woodhouse
2008-04-24 10:14                                 ` Alexander Belyakov
2008-04-24 10:32                                   ` David Woodhouse
2008-04-24 10:42                                     ` Alexander Belyakov
2008-04-24 11:05                                       ` David Woodhouse
2008-04-24 11:11                                         ` Jörn Engel
2008-04-24 11:21                                         ` Alexander Belyakov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox