Patchwork [JFFS2] kernel BUG at fs/jffs2/gc.c:516!

login
register
mail settings
Submitter David Woodhouse
Date Oct. 28, 2008, 10:09 a.m.
Message ID <1225188544.18031.172.camel@macbook.infradead.org>
Download mbox | patch
Permalink /patch/6073/
State New
Headers show

Comments

David Woodhouse - Oct. 28, 2008, 10:09 a.m.
On Wed, 2008-10-22 at 16:42 +0900, Kyungmin Park wrote:
> Hi,
> 
> On Wed, Oct 22, 2008 at 3:38 PM, Brijesh Singh
> <brijesh.s.singh@gmail.com> wrote:
> > Hi,
> >     I was testing JFFS2 on 2.6.20 with fsstress for OneNand Chip. JFFS2 died
> > with the a BUG in gc.c.
> >
> >      I have gone through all  the mailing archive possible. Kyungmin park
> > had reported the problem,
> >  but I could not find the fix.
> >
> >      Here is the mail:
> > http://lists.infradead.org/pipermail/linux-mtd/2007-April/017979.html
> >
> >      Any idea,why this BUG occurs/occurred? Is there any fix available?
> 
> You maybe test the fsstress on it.
> After that time, no more test. Could you can test the latest kernel?
> In Flex-OneNAND test, it's not occurred as I know. It means it's
> solved with some patch or other side effect?

Does this fix it?
Brijesh Singh - Nov. 5, 2008, 4:53 a.m.
Hi David,
  Sorry for the delayed response.

>> >      I have gone through all  the mailing archive possible. Kyungmin park
>> > had reported the problem,
>> >  but I could not find the fix.
>> >
>> >      Here is the mail:
>> > http://lists.infradead.org/pipermail/linux-mtd/2007-April/017979.html
>> >
>> >      Any idea,why this BUG occurs/occurred? Is there any fix available?
>>

>
> Does this fix it?
>
> diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
> index 8adebd3..3cceef4 100644
> --- a/fs/jffs2/background.c
> +++ b/fs/jffs2/background.c
> @@ -85,15 +85,15 @@ static int jffs2_garbage_collect_thread(void *_c)
>        for (;;) {
>                allow_signal(SIGHUP);
>        again:
> +               spin_lock(&c->erase_completion_lock);
>                if (!jffs2_thread_should_wake(c)) {
>                        set_current_state (TASK_INTERRUPTIBLE);
> +                       spin_unlock(&c->erase_completion_lock);
>                        D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread sleeping...\n"));
> -                       /* Yes, there's a race here; we checked jffs2_thread_should_wake()
> -                          before setting current->state to TASK_INTERRUPTIBLE. But it doesn't
> -                          matter - We don't care if we miss a wakeup, because the GC thread
> -                          is only an optimisation anyway. */
>                        schedule();
> -               }
> +               } else
> +                       spin_unlock(&c->erase_completion_lock);
> +
>
>                /* This thread is purely an optimisation. But if it runs when
>                   other things could be running, it actually makes things a
>
> --
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@intel.com                              Intel Corporation

Yes this patch fixed the problem.I have tested with fsstress for 2
days.I could not reproduce the BUG again.


Thanks,
Brijesh

Patch

diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
index 8adebd3..3cceef4 100644
--- a/fs/jffs2/background.c
+++ b/fs/jffs2/background.c
@@ -85,15 +85,15 @@  static int jffs2_garbage_collect_thread(void *_c)
 	for (;;) {
 		allow_signal(SIGHUP);
 	again:
+		spin_lock(&c->erase_completion_lock);
 		if (!jffs2_thread_should_wake(c)) {
 			set_current_state (TASK_INTERRUPTIBLE);
+			spin_unlock(&c->erase_completion_lock);
 			D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread sleeping...\n"));
-			/* Yes, there's a race here; we checked jffs2_thread_should_wake()
-			   before setting current->state to TASK_INTERRUPTIBLE. But it doesn't
-			   matter - We don't care if we miss a wakeup, because the GC thread
-			   is only an optimisation anyway. */
 			schedule();
-		}
+		} else
+			spin_unlock(&c->erase_completion_lock);
+			
 
 		/* This thread is purely an optimisation. But if it runs when
 		   other things could be running, it actually makes things a