Revert "jffs2: Fix lock acquisition order bug in jffs2_write_begin"

commit 157078f64b8a9cd7011b6b900b2f2498df850748 upstream.

This reverts commit 5ffd3412ae55
("jffs2: Fix lock acquisition order bug in jffs2_write_begin").

The commit modified jffs2_write_begin() to remove a deadlock with
jffs2_garbage_collect_live(), but this introduced new deadlocks found
by multiple users. page_lock() actually has to be called before
mutex_lock(&c->alloc_sem) or mutex_lock(&f->sem) because
jffs2_write_end() and jffs2_readpage() are called with the page locked,
and they acquire c->alloc_sem and f->sem, resp.

In other words, the lock order in jffs2_write_begin() was correct, and
it is the jffs2_garbage_collect_live() path that has to be changed.

Revert the commit to get rid of the new deadlocks, and to clear the way
for a better fix of the original deadlock.

Reported-by: Deng Chao <deng.chao1@zte.com.cn>
Reported-by: Ming Liu <liu.ming50@gmail.com>
Reported-by: wangzaiwei <wangzaiwei@top-vision.cn>
Signed-off-by: Thomas Betker <thomas.betker@rohde-schwarz.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
[bwh: Backported to 3.2: use D1(printk(KERN_DEBUG ...)) instead of
 jffs2_dbg(1, ...)]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index 0095a70..4964a5b 100644 (file)
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -135,39 +135,33 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
        struct page *pg;
        struct inode *inode = mapping->host;
        struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
-       struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
-       struct jffs2_raw_inode ri;
-       uint32_t alloc_len = 0;
        pgoff_t index = pos >> PAGE_CACHE_SHIFT;
        uint32_t pageofs = index << PAGE_CACHE_SHIFT;
        int ret = 0;
 
-       D1(printk(KERN_DEBUG "%s()\n", __func__));
-
-       if (pageofs > inode->i_size) {
-               ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len,
-                                         ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE);
-               if (ret)
-                       return ret;
-       }
-
-       mutex_lock(&f->sem);
        pg = grab_cache_page_write_begin(mapping, index, flags);
-       if (!pg) {
-               if (alloc_len)
-                       jffs2_complete_reservation(c);
-               mutex_unlock(&f->sem);
+       if (!pg)
                return -ENOMEM;
-       }
        *pagep = pg;
 
-       if (alloc_len) {
+       D1(printk(KERN_DEBUG "%s()\n", __func__));
+
+       if (pageofs > inode->i_size) {
                /* Make new hole frag from old EOF to new page */
+               struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
+               struct jffs2_raw_inode ri;
                struct jffs2_full_dnode *fn;
+               uint32_t alloc_len;
 
                D1(printk(KERN_DEBUG "Writing new hole frag 0x%x-0x%x between current EOF and new page\n",
                          (unsigned int)inode->i_size, pageofs));
 
+               ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len,
+                                         ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE);
+               if (ret)
+                       goto out_page;
+
+               mutex_lock(&f->sem);
                memset(&ri, 0, sizeof(ri));
 
                ri.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK);
@@ -194,6 +188,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
                if (IS_ERR(fn)) {
                        ret = PTR_ERR(fn);
                        jffs2_complete_reservation(c);
+                       mutex_unlock(&f->sem);
                        goto out_page;
                }
                ret = jffs2_add_full_dnode_to_inode(c, f, fn);
@@ -207,10 +202,12 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
                        jffs2_mark_node_obsolete(c, fn->raw);
                        jffs2_free_full_dnode(fn);
                        jffs2_complete_reservation(c);
+                       mutex_unlock(&f->sem);
                        goto out_page;
                }
                jffs2_complete_reservation(c);
                inode->i_size = pageofs;
+               mutex_unlock(&f->sem);
        }
 
        /*
@@ -219,18 +216,18 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
         * case of a short-copy.
         */
        if (!PageUptodate(pg)) {
+               mutex_lock(&f->sem);
                ret = jffs2_do_readpage_nolock(inode, pg);
+               mutex_unlock(&f->sem);
                if (ret)
                        goto out_page;
        }
-       mutex_unlock(&f->sem);
        D1(printk(KERN_DEBUG "end write_begin(). pg->flags %lx\n", pg->flags));
        return ret;
 
 out_page:
        unlock_page(pg);
        page_cache_release(pg);
-       mutex_unlock(&f->sem);
        return ret;
 }