Btrfs: Direct I/O: Fix space accounting

The following call trace is seen when generic/095 test is executed,

WARNING: CPU: 3 PID: 2769 at /home/chandan/code/repos/linux/fs/btrfs/inode.c:8967 btrfs_destroy_inode+0x284/0x2a0()
Modules linked in:
CPU: 3 PID: 2769 Comm: umount Not tainted 4.2.0-rc5+ #31
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20150306_163512-brownie 04/01/2014
 ffffffff81c08150 ffff8802ec9cbce8 ffffffff81984058 ffff8802ffd8feb0
 0000000000000000 ffff8802ec9cbd28 ffffffff81050385 ffff8802ec9cbd38
 ffff8802d12f8588 ffff8802d12f8588 ffff8802f15ab000 ffff8800bb96c0b0
Call Trace:
 [<ffffffff81984058>] dump_stack+0x45/0x57
 [<ffffffff81050385>] warn_slowpath_common+0x85/0xc0
 [<ffffffff81050465>] warn_slowpath_null+0x15/0x20
 [<ffffffff81340294>] btrfs_destroy_inode+0x284/0x2a0
 [<ffffffff8117ce07>] destroy_inode+0x37/0x60
 [<ffffffff8117cf39>] evict+0x109/0x170
 [<ffffffff8117cfd5>] dispose_list+0x35/0x50
 [<ffffffff8117dd3a>] evict_inodes+0xaa/0x100
 [<ffffffff81165667>] generic_shutdown_super+0x47/0xf0
 [<ffffffff81165951>] kill_anon_super+0x11/0x20
 [<ffffffff81302093>] btrfs_kill_super+0x13/0x110
 [<ffffffff81165c99>] deactivate_locked_super+0x39/0x70
 [<ffffffff811660cf>] deactivate_super+0x5f/0x70
 [<ffffffff81180e1e>] cleanup_mnt+0x3e/0x90
 [<ffffffff81180ebd>] __cleanup_mnt+0xd/0x10
 [<ffffffff81069c06>] task_work_run+0x96/0xb0
 [<ffffffff81003a3d>] do_notify_resume+0x3d/0x50
 [<ffffffff8198cbc2>] int_signal+0x12/0x17

This means that the inode had non-zero "outstanding extents" during
eviction. This occurs because, during direct I/O a task which successfully
used up its reserved data space would set BTRFS_INODE_DIO_READY bit and does
not clear the bit after finishing the DIO write. A future DIO write could
actually fail and the unused reserve space won't be freed because of the
previously set BTRFS_INODE_DIO_READY bit.

Clearing the BTRFS_INODE_DIO_READY bit in btrfs_direct_IO() caused the
following issue,
|-----------------------------------+-------------------------------------|
| Task A                            | Task B                              |
|-----------------------------------+-------------------------------------|
| Start direct i/o write on inode X.|                                     |
| reserve space                     |                                     |
| Allocate ordered extent           |                                     |
| release reserved space            |                                     |
| Set BTRFS_INODE_DIO_READY bit.    |                                     |
|                                   | splice()                            |
|                                   | Transfer data from pipe buffer to   |
|                                   | destination file.                   |
|                                   | - kmap(pipe buffer page)            |
|                                   | - Start direct i/o write on         |
|                                   |   inode X.                          |
|                                   |   - reserve space                   |
|                                   |   - dio_refill_pages()              |
|                                   |     - sdio->blocks_available == 0   |
|                                   |     - Since a kernel address is     |
|                                   |       being passed instead of a     |
|                                   |       user space address,           |
|                                   |       iov_iter_get_pages() returns  |
|                                   |       -EFAULT.                      |
|                                   |   - Since BTRFS_INODE_DIO_READY is  |
|                                   |     set, we don't release reserved  |
|                                   |     space.                          |
|                                   |   - Clear BTRFS_INODE_DIO_READY bit.|
| -EIOCBQUEUED is returned.         |                                     |
|-----------------------------------+-------------------------------------|

Hence this commit introduces "struct btrfs_dio_data" to track the usage of
reserved data space. The remaining unused "reserve space" can now be freed
reliably.

Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
Reviewed-by: Liu Bo <bo.li.liu@oracle.com>
Signed-off-by: Chris Mason <clm@fb.com>

2 files changed, 21 insertions, 23 deletions

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 81220b2203c6..0ef5cc13fae2 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -44,8 +44,6 @@
 #define BTRFS_INODE_IN_DELALLOC_LIST            9
 #define BTRFS_INODE_READDIO_NEED_LOCK           10
 #define BTRFS_INODE_HAS_PROPS                   11
-/* DIO is ready to submit */
-#define BTRFS_INODE_DIO_READY                   12
 /*
  * The following 3 bits are meant only for the btree inode.
  * When any of them is set, it means an error happened while writing an
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3d983dea57af..b7e439bf5e4f 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7405,6 +7405,10 @@ static struct extent_map *create_pinned_em(struct inode *inode, u64 start,
         return em;
 }
 
+struct btrfs_dio_data {
+        u64 outstanding_extents;
+        u64 reserve;
+};
 
 static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
                                    struct buffer_head *bh_result, int create)
@@ -7412,10 +7416,10 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
         struct extent_map *em;
         struct btrfs_root *root = BTRFS_I(inode)->root;
         struct extent_state *cached_state = NULL;
+        struct btrfs_dio_data *dio_data = NULL;
         u64 start = iblock << inode->i_blkbits;
         u64 lockstart, lockend;
         u64 len = bh_result->b_size;
-        u64 *outstanding_extents = NULL;
         int unlock_bits = EXTENT_LOCKED;
         int ret = 0;
 
@@ -7433,7 +7437,7 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
                  * that anything that needs to check if there's a transction doesn't get
                  * confused.
                  */
-                outstanding_extents = current->journal_info;
+                dio_data = current->journal_info;
                 current->journal_info = NULL;
         }
 
@@ -7565,17 +7569,18 @@ unlock:
                  * within our reservation, otherwise we need to adjust our inode
                  * counter appropriately.
                  */
-                if (*outstanding_extents) {
-                        (*outstanding_extents)--;
+                if (dio_data->outstanding_extents) {
+                        (dio_data->outstanding_extents)--;
                 } else {
                         spin_lock(&BTRFS_I(inode)->lock);
                         BTRFS_I(inode)->outstanding_extents++;
                         spin_unlock(&BTRFS_I(inode)->lock);
                 }
 
-                current->journal_info = outstanding_extents;
                 btrfs_free_reserved_data_space(inode, len);
-                set_bit(BTRFS_INODE_DIO_READY, &BTRFS_I(inode)->runtime_flags);
+                WARN_ON(dio_data->reserve < len);
+                dio_data->reserve -= len;
+                current->journal_info = dio_data;
         }
 
         /*
@@ -7598,8 +7603,8 @@ unlock:
 unlock_err:
         clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart, lockend,
                          unlock_bits, 1, 0, &cached_state, GFP_NOFS);
-        if (outstanding_extents)
-                current->journal_info = outstanding_extents;
+        if (dio_data)
+                current->journal_info = dio_data;
         return ret;
 }
 
@@ -8329,7 +8334,8 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 {
         struct file *file = iocb->ki_filp;
         struct inode *inode = file->f_mapping->host;
-        u64 outstanding_extents = 0;
+        struct btrfs_root *root = BTRFS_I(inode)->root;
+        struct btrfs_dio_data dio_data = { 0 };
         size_t count = 0;
         int flags = 0;
         bool wakeup = true;
@@ -8367,7 +8373,7 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
                 ret = btrfs_delalloc_reserve_space(inode, count);
                 if (ret)
                         goto out;
-                outstanding_extents = div64_u64(count +
+                dio_data.outstanding_extents = div64_u64(count +
                                                 BTRFS_MAX_EXTENT_SIZE - 1,
                                                 BTRFS_MAX_EXTENT_SIZE);
 
@@ -8376,7 +8382,8 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
                  * do the accounting properly if we go over the number we
                  * originally calculated.  Abuse current->journal_info for this.
                  */
-                current->journal_info = &outstanding_extents;
+                dio_data.reserve = round_up(count, root->sectorsize);
+                current->journal_info = &dio_data;
         } else if (test_bit(BTRFS_INODE_READDIO_NEED_LOCK,
                                      &BTRFS_I(inode)->runtime_flags)) {
                 inode_dio_end(inode);
@@ -8391,16 +8398,9 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
         if (iov_iter_rw(iter) == WRITE) {
                 current->journal_info = NULL;
                 if (ret < 0 && ret != -EIOCBQUEUED) {
-                        /*
-                         * If the error comes from submitting stage,
-                         * btrfs_get_blocsk_direct() has free'd data space,
-                         * and metadata space will be handled by
-                         * finish_ordered_fn, don't do that again to make
-                         * sure bytes_may_use is correct.
-                         */
-                        if (!test_and_clear_bit(BTRFS_INODE_DIO_READY,
-                                     &BTRFS_I(inode)->runtime_flags))
-                                btrfs_delalloc_release_space(inode, count);
+                        if (dio_data.reserve)
+                                btrfs_delalloc_release_space(inode,
+                                                        dio_data.reserve);
                 } else if (ret >= 0 && (size_t)ret < count)
                         btrfs_delalloc_release_space(inode,
                                                      count - (size_t)ret);