f2fs: fix race on allocating and deallocating a dentry block
authorJaegeuk Kim <jaegeuk@kernel.org>
Wed, 29 Apr 2015 18:18:42 +0000 (11:18 -0700)
committerJaegeuk Kim <jaegeuk@kernel.org>
Thu, 28 May 2015 22:41:35 +0000 (15:41 -0700)
There are two threads:
 f2fs_delete_entry()              get_new_data_page()
                                  f2fs_reserve_block()
  dn.blkaddr = XXX
 lock_page(dentry_block)
 truncate_hole()
 dn.blkaddr = NULL
 unlock_page(dentry_block)
                                  lock_page(dentry_block)
                                  fill the block from XXX address
                                  add new dentries
                                  unlock_page(dentry_block)

Later, f2fs_write_data_page() will truncate the dentry_block, since
its block address is NULL.

The reason for this was due to the wrong lock order.
In this case, we should do f2fs_reserve_block() after locking its dentry block.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
fs/f2fs/data.c

index 81d1fd5..9ba30b4 100644 (file)
@@ -1076,20 +1076,22 @@ struct page *get_new_data_page(struct inode *inode,
        struct page *page;
        struct dnode_of_data dn;
        int err;
+repeat:
+       page = grab_cache_page(mapping, index);
+       if (!page)
+               return ERR_PTR(-ENOMEM);
 
        set_new_dnode(&dn, inode, ipage, NULL, 0);
        err = f2fs_reserve_block(&dn, index);
-       if (err)
+       if (err) {
+               f2fs_put_page(page, 1);
                return ERR_PTR(err);
-repeat:
-       page = grab_cache_page(mapping, index);
-       if (!page) {
-               err = -ENOMEM;
-               goto put_err;
        }
+       if (!ipage)
+               f2fs_put_dnode(&dn);
 
        if (PageUptodate(page))
-               return page;
+               goto got_it;
 
        if (dn.data_blkaddr == NEW_ADDR) {
                zero_user_segment(page, 0, PAGE_CACHE_SIZE);
@@ -1104,20 +1106,19 @@ repeat:
                };
                err = f2fs_submit_page_bio(&fio);
                if (err)
-                       goto put_err;
+                       return ERR_PTR(err);
 
                lock_page(page);
                if (unlikely(!PageUptodate(page))) {
                        f2fs_put_page(page, 1);
-                       err = -EIO;
-                       goto put_err;
+                       return ERR_PTR(-EIO);
                }
                if (unlikely(page->mapping != mapping)) {
                        f2fs_put_page(page, 1);
                        goto repeat;
                }
        }
-
+got_it:
        if (new_i_size &&
                i_size_read(inode) < ((index + 1) << PAGE_CACHE_SHIFT)) {
                i_size_write(inode, ((index + 1) << PAGE_CACHE_SHIFT));
@@ -1125,10 +1126,6 @@ repeat:
                set_inode_flag(F2FS_I(inode), FI_UPDATE_DIR);
        }
        return page;
-
-put_err:
-       f2fs_put_dnode(&dn);
-       return ERR_PTR(err);
 }
 
 static int __allocate_data_block(struct dnode_of_data *dn)