Patchwork jffs2: Fix serious write stall due to erase

login
register
mail settings
Submitter Joakim Tjernlund
Date Oct. 7, 2010, 4:01 p.m.
Message ID <1286467304-23370-1-git-send-email-Joakim.Tjernlund@transmode.se>
Download mbox | patch
Permalink /patch/67072/
State New
Headers show

Comments

Joakim Tjernlund - Oct. 7, 2010, 4:01 p.m.
Drop the alloc_sem before erasing flash in
jffs2_garbage_collect_pass().
Otherwise writes are put on hold until the erase
has finised.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---

 I am fairly sure it is safe to unlock the alloc_sem before
 calling jffs2_erase_pending_blocks(). Extra confirmation
 would be great though.

 This problem was introduced in 2.6.35

 fs/jffs2/gc.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Joakim Tjernlund - Oct. 9, 2010, 4:36 p.m.
David, pretty please :)

  Jocke
>
> Drop the alloc_sem before erasing flash in
> jffs2_garbage_collect_pass().
> Otherwise writes are put on hold until the erase
> has finised.
>
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>
>  I am fairly sure it is safe to unlock the alloc_sem before
>  calling jffs2_erase_pending_blocks(). Extra confirmation
>  would be great though.
>
>  This problem was introduced in 2.6.35
>
>  fs/jffs2/gc.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
> index f5e96bd..b5f7175 100644
> --- a/fs/jffs2/gc.c
> +++ b/fs/jffs2/gc.c
> @@ -218,13 +218,14 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c)
>     if (!list_empty(&c->erase_complete_list) ||
>         !list_empty(&c->erase_pending_list)) {
>        spin_unlock(&c->erase_completion_lock);
> +      mutex_unlock(&c->alloc_sem);
>        D1(printk(KERN_DEBUG "jffs2_garbage_collect_pass() erasing pending blocks\n"));
> -      if (jffs2_erase_pending_blocks(c, 1)) {
> -         mutex_unlock(&c->alloc_sem);
> +      if (jffs2_erase_pending_blocks(c, 1))
>           return 0;
> -      }
> +
>        D1(printk(KERN_DEBUG "No progress from erasing blocks; doing GC anyway\n"));
>        spin_lock(&c->erase_completion_lock);
> +      mutex_lock(&c->alloc_sem);
>     }
>
>     /* First, work out which block we're garbage-collecting */
> --
> 1.7.2.2
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Artem Bityutskiy - Oct. 12, 2010, 8:42 a.m.
On Thu, 2010-10-07 at 18:01 +0200, Joakim Tjernlund wrote:
> Drop the alloc_sem before erasing flash in
> jffs2_garbage_collect_pass().
> Otherwise writes are put on hold until the erase
> has finised.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
> 
>  I am fairly sure it is safe to unlock the alloc_sem before
>  calling jffs2_erase_pending_blocks(). Extra confirmation
>  would be great though.
> 
>  This problem was introduced in 2.6.35

This looks safe for me as well, but I do not remember JFFS2 locking
rules anymore, so I'm not 100% sure. I've pushed your patch to my
l2-mtd-2.6.git.
Joakim Tjernlund - Oct. 12, 2010, 8:55 a.m.
Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/10/12 10:42:39:
>
> On Thu, 2010-10-07 at 18:01 +0200, Joakim Tjernlund wrote:
> > Drop the alloc_sem before erasing flash in
> > jffs2_garbage_collect_pass().
> > Otherwise writes are put on hold until the erase
> > has finised.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >
> >  I am fairly sure it is safe to unlock the alloc_sem before
> >  calling jffs2_erase_pending_blocks(). Extra confirmation
> >  would be great though.
> >
> >  This problem was introduced in 2.6.35
>
> This looks safe for me as well, but I do not remember JFFS2 locking
> rules anymore, so I'm not 100% sure. I've pushed your patch to my
> l2-mtd-2.6.git.

Thanks, now if just David could weigh in :)

      Jocke

Patch

diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
index f5e96bd..b5f7175 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -218,13 +218,14 @@  int jffs2_garbage_collect_pass(struct jffs2_sb_info *c)
 	if (!list_empty(&c->erase_complete_list) ||
 	    !list_empty(&c->erase_pending_list)) {
 		spin_unlock(&c->erase_completion_lock);
+		mutex_unlock(&c->alloc_sem);
 		D1(printk(KERN_DEBUG "jffs2_garbage_collect_pass() erasing pending blocks\n"));
-		if (jffs2_erase_pending_blocks(c, 1)) {
-			mutex_unlock(&c->alloc_sem);
+		if (jffs2_erase_pending_blocks(c, 1))
 			return 0;
-		}
+
 		D1(printk(KERN_DEBUG "No progress from erasing blocks; doing GC anyway\n"));
 		spin_lock(&c->erase_completion_lock);
+		mutex_lock(&c->alloc_sem);
 	}
 
 	/* First, work out which block we're garbage-collecting */