Re: JFFS2 deadlock with alloc_sem

["ye janboe" <janboe.ye@gmail.com>]

David

Thanks for you comments.
I still can not understand why there is comment in code say the dead
lock can not happen.
Could you give me some hints?

2007/7/27, Dave Kleikamp <shaggy@linux.vnet.ibm.com>:
> On Fri, 2007-07-27 at 11:38 +0800, ye janboe wrote:
> > Does this make fsync system call failed to cause a risk of loss data?
>
> Could be.  It looks like I wasn't careful about considering all the
> callers of jffs2_garbage_collect_pass().  I assume the problem is when
> it is called from jffs2_flush_wbuf_gc().
>
> > I posted my fix base on your patch, please review it.
> >
> > Thanks
> >
> > --- gc.c@@/main/fl71_main/a22116_jffs2_sync_debug/0     2007-07-24
> > 17:02:26.746154000 +0800
> > +++ gc.c@@/main/fl71_main/a22116_jffs2_sync_debug/2     2007-07-24
> > 17:39:35.000000000 +0800
> > @@ -1299,7 +1299,10 @@ static int jffs2_garbage_collect_dnode(s
> >         if (IS_ERR(pg_ptr)) {
> >                 if ( PTR_ERR(pg_ptr) == -EBUSY ) {
> >                         printk(KERN_WARNING "jffs2_gc_fetch_page() rc
> > -EBUSY. Deadlock avoided.\n" );
> > -                       return 0;
> > +                       if (current->flags & PF_SYNCWRITE)
> > +                               return -EBUSY;
> > +                       else
> > +                               return 0;
> >                 } else {
> >                         printk(KERN_WARNING "jffs2_gc_fetch_page() rc
> > %ld\n", PTR_ERR(pg_ptr));
> >                         return PTR_ERR(pg_ptr);
>
> That is probably okay, but I think it is getting more complicated than
> it needs to be.
>
> What if we let jffs2_garbage_collect_dnode() return -EBUSY all the time,
> and let the callers that don't care ignore it?
>
> How's this look?
>
> diff -Nurp linux.orig/fs/jffs2/gc.c linux/fs/jffs2/gc.c
> --- linux.orig/fs/jffs2/gc.c    2007-07-27 08:32:36.000000000 -0500
> +++ linux/fs/jffs2/gc.c 2007-07-27 08:33:59.000000000 -0500
> @@ -1221,13 +1221,8 @@ static int jffs2_garbage_collect_dnode(s
>         pg_ptr = jffs2_gc_fetch_page(c, f, start, &pg);
>
>         if (IS_ERR(pg_ptr)) {
> -               if ( PTR_ERR(pg_ptr) == -EBUSY ) {
> -                       printk(KERN_WARNING "jffs2_gc_fetch_page() rc -EBUSY. Deadlock avoided.\n" );
> -                       return 0;
> -               } else {
> -                       printk(KERN_WARNING "jffs2_gc_fetch_page() rc %ld\n", PTR_ERR(pg_ptr));
> -                       return PTR_ERR(pg_ptr);
> -               }
> +               printk(KERN_WARNING "jffs2_gc_fetch_page() rc %ld\n", PTR_ERR(pg_ptr));
> +               return PTR_ERR(pg_ptr);
>         }
>
>         offset = start;
> diff -Nurp linux.orig/fs/jffs2/nodemgmt.c linux/fs/jffs2/nodemgmt.c
> --- linux.orig/fs/jffs2/nodemgmt.c      2007-07-27 08:32:25.000000000 -0500
> +++ linux/fs/jffs2/nodemgmt.c   2007-07-27 08:35:46.000000000 -0500
> @@ -117,7 +117,7 @@ int jffs2_reserve_space(struct jffs2_sb_
>                         spin_unlock(&c->erase_completion_lock);
>
>                         ret = jffs2_garbage_collect_pass(c);
> -                       if (ret)
> +                       if (ret && (ret != -EBUSY))
>                                 return ret;
>
>                         cond_resched();
>
> --
> David Kleikamp
> IBM Linux Technology Center
>
>