JFFS2 appears to "freeze" during erase

[Radoslaw Bisewski]

I'm sending this message once again - this time in plain text format.
Could someone review these changes before putting them into the MTD?
With current desing erase_free_sem is locked every time the flash
block is being erased. For NOR flashes - ~1 second is needed to erase
single flash block. In the worst case scenario erase_free_sem may be
locked for a couple of seconds when the number of blocks is being
erased (e.g. after large file was removed). When erase_free_sem is
locked all read/write operations for given JFFS2 partition are locked
too - in effect from time to time access to the JFFS2 partition is
locked for a number of seconds. This fix makes critical section in
flash erasing procedure shorter - now erase_free_sem is locked around
erase_completion_lock spinlock only.

Regards
R.

On 9/29/06, Joakim Tjernlund <joakim.tjernlund at transmode.se> wrote:
> >
> >
> >
> > On 9/21/06, Joakim Tjernlund <Joakim.Tjernlund at transmode.se > wrote:
> >
> >       Radoslaw Bisewski wrote:
> >       > This bug is caused by the use of erase_free_sem
> > semaphore. Block erasing
> >       > function jffs2_erase_block is called with
> > erase_free_sem locked, i.e.
> >       > erase_free_sem will be locked for the time needed to
> > erase flash block (
> >       > ~1 s for typical NOR chip), in the worst scenario
> > access to file system
> >       > may be blocked for a numer of seconds when the number
> > of blocks is being
> >       > erased. Workaround for this bug is simple -
> > erase_free_sem cannot be
> >       > locked when jffs2_erase_block function is called. See
> > attached patch.
> >       >
> >       Me again
> >
> >       I think there is a bug in jffs2_erase_pending_blocks()
> > after applying your patch:
> >              spin_unlock(&c->erase_completion_lock);
> >       done:
> >              D1(printk(KERN_DEBUG "jffs2_erase_pending_blocks
> > completed\n"));
> >
> >              up(&c->erase_free_sem);
> >
> >       the up(&c->erase_free_sem); needs to move to before done:
> >
> >       Jocke
> >
> >
> >
> > Joakim,
> >
> > yes, you are right. attached is the corrected version.
> >
> > Regards
> > R.
> >
>
> Why isn't this patch in MTD yet?
> David, can you please comment?
>
>  Jcoke
>
-------------- next part --------------
--- erase.c	2006-09-21 10:21:48.000000000 +0200
+++ erase-fixed.c	2006-09-21 14:24:23.000000000 +0200
@@ -53,6 +53,7 @@
 	instr = kmalloc(sizeof(struct erase_info) + sizeof(struct erase_priv_struct), GFP_KERNEL);
 	if (!instr) {
 		printk(KERN_WARNING "kmalloc for struct erase_info in jffs2_erase_block failed. Refiling block for later\n");
+        down(&c->erase_free_sem);        
 		spin_lock(&c->erase_completion_lock);
 		list_del(&jeb->list);
 		list_add(&jeb->list, &c->erase_pending_list);
@@ -60,6 +61,7 @@
 		c->dirty_size += c->sector_size;
 		jeb->dirty_size = c->sector_size;
 		spin_unlock(&c->erase_completion_lock);
+        up(&c->erase_free_sem);
 		return;
 	}
 
@@ -86,13 +88,15 @@
 	if (ret == -ENOMEM || ret == -EAGAIN) {
 		/* Erase failed immediately. Refile it on the list */
 		D1(printk(KERN_DEBUG "Erase at 0x%08x failed: %d. Refiling on erase_pending_list\n", jeb->offset, ret));
-		spin_lock(&c->erase_completion_lock);
+        down(&c->erase_free_sem);
+        spin_lock(&c->erase_completion_lock);
 		list_del(&jeb->list);
 		list_add(&jeb->list, &c->erase_pending_list);
 		c->erasing_size -= c->sector_size;
 		c->dirty_size += c->sector_size;
 		jeb->dirty_size = c->sector_size;
 		spin_unlock(&c->erase_completion_lock);
+        up(&c->erase_free_sem);
 		return;
 	}
 
@@ -119,7 +123,8 @@
 			jeb = list_entry(c->erase_complete_list.next, struct jffs2_eraseblock, list);
 			list_del(&jeb->list);
 			spin_unlock(&c->erase_completion_lock);
-			jffs2_mark_erased_block(c, jeb);
+                        up(&c->erase_free_sem);
+                        jffs2_mark_erased_block(c, jeb);
 
 			if (!--count) {
 				D1(printk(KERN_DEBUG "Count reached. jffs2_erase_pending_blocks leaving\n"));
@@ -139,6 +144,7 @@
 			jffs2_free_all_node_refs(c, jeb);
 			list_add(&jeb->list, &c->erasing_list);
 			spin_unlock(&c->erase_completion_lock);
+                        up(&c->erase_free_sem);
 
 			jffs2_erase_block(c, jeb);
 
@@ -148,14 +154,15 @@
 
 		/* Be nice */
 		cond_resched();
+        
+                down(&c->erase_free_sem);
 		spin_lock(&c->erase_completion_lock);
 	}
 
 	spin_unlock(&c->erase_completion_lock);
+        up(&c->erase_free_sem);
  done:
 	D1(printk(KERN_DEBUG "jffs2_erase_pending_blocks completed\n"));
-
-	up(&c->erase_free_sem);
 }
 
 static void jffs2_erase_succeeded(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb)
@@ -425,6 +432,8 @@
 		jeb->wasted_size = 0;
 	}
 
+	down(&c->erase_free_sem);
+
 	spin_lock(&c->erase_completion_lock);
 	c->erasing_size -= c->sector_size;
 	c->free_size += jeb->free_size;
@@ -438,22 +447,28 @@
 	c->nr_free_blocks++;
 	spin_unlock(&c->erase_completion_lock);
 	wake_up(&c->erase_wait);
+    	up(&c->erase_free_sem);
+
 	return;
 
 filebad:
+   	down(&c->erase_free_sem);
 	spin_lock(&c->erase_completion_lock);
 	/* Stick it on a list (any list) so erase_failed can take it
 	   right off again.  Silly, but shouldn't happen often. */
 	list_add(&jeb->list, &c->erasing_list);
 	spin_unlock(&c->erase_completion_lock);
+    up(&c->erase_free_sem);
 	jffs2_erase_failed(c, jeb, bad_offset);
 	return;
 
 refile:
 	/* Stick it back on the list from whence it came and come back later */
 	jffs2_erase_pending_trigger(c);
+	down(&c->erase_free_sem);    
 	spin_lock(&c->erase_completion_lock);
 	list_add(&jeb->list, &c->erase_complete_list);
 	spin_unlock(&c->erase_completion_lock);
+	up(&c->erase_free_sem);    
 	return;
 }