* Re: JFFS2 appears to "freeze" during erase
[not found] <F6AD7E21CDF4E145A44F61F43EE6D9399B4CB1@tmnt04.transmode.se>
@ 2006-10-02 13:40 ` Radoslaw Bisewski
0 siblings, 0 replies; 4+ messages in thread
From: Radoslaw Bisewski @ 2006-10-02 13:40 UTC (permalink / raw)
To: linux-mtd; +Cc: Joakim Tjernlund
[-- Attachment #1: Type: text/plain, Size: 2229 bytes --]
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@transmode.se> wrote:
> >
> >
> >
> > On 9/21/06, Joakim Tjernlund <Joakim.Tjernlund@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
>
[-- Attachment #2: jffs2-erase-patch.patch --]
[-- Type: text/plain, Size: 3441 bytes --]
--- 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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread