Btrfs: fix reported number of inode blocks after buffered append writes
authorFilipe Manana <fdmanana@suse.com>
Sat, 4 Nov 2017 00:16:59 +0000 (00:16 +0000)
committerDavid Sterba <dsterba@suse.com>
Wed, 15 Nov 2017 16:27:46 +0000 (17:27 +0100)
The patch from commit a7e3b975a0f9 ("Btrfs: fix reported number of inode
blocks") introduced a regression where if we do a buffered write starting
at position equal to or greater than the file's size and then stat(2) the
file before writeback is triggered, the number of used blocks does not
change (unless there's a prealloc/unwritten extent). Example:

  $ xfs_io -f -c "pwrite -S 0xab 0 64K" foobar
  $ du -h foobar
  0 foobar
  $ sync
  $ du -h foobar
  64K foobar

The first version of that patch didn't had this regression and the second
version, which was the one committed, was made only to address some
performance regression detected by the intel test robots using fs_mark.

This fixes the regression by setting the new delaloc bit in the range, and
doing it at btrfs_dirty_pages() while setting the regular dealloc bit as
well, so that this way we set both bits at once avoiding navigation of the
inode's io tree twice. Doing it at btrfs_dirty_pages() is also the most
meaninful place, as we should set the new dellaloc bit when if we set the
delalloc bit, which happens only if we copied bytes into the pages at
__btrfs_buffered_write().

This was making some of LTP's du tests fail, which can be quickly run
using a command line like the following:

  $ ./runltp -q -p -l /ltp.log -f commands -s du -d /mnt

Fixes: a7e3b975a0f9 ("Btrfs: fix reported number of inode blocks")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/ctree.h
fs/btrfs/extent_io.h
fs/btrfs/file.c
fs/btrfs/inode.c
fs/btrfs/relocation.c
fs/btrfs/tests/extent-io-tests.c
fs/btrfs/tests/inode-tests.c

index f7df5536ab61e1f6de0512328341c5a637d040d3..72dcbf19f6cef12b2650ae0ce410aa70cf99935b 100644 (file)
@@ -3180,6 +3180,7 @@ int btrfs_start_delalloc_inodes(struct btrfs_root *root, int delay_iput);
 int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int delay_iput,
                               int nr);
 int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
+                             unsigned int extra_bits,
                              struct extent_state **cached_state, int dedupe);
 int btrfs_create_subvol_root(struct btrfs_trans_handle *trans,
                             struct btrfs_root *new_root,
index d8b27af7101e6fb6b15961c4b3bf4c640e30222a..2ef824b58e3c73bc6dad79ca77cda5b54ab4ff7c 100644 (file)
@@ -365,10 +365,11 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
                       struct extent_state **cached_state);
 
 static inline int set_extent_delalloc(struct extent_io_tree *tree, u64 start,
-               u64 end, struct extent_state **cached_state)
+                                     u64 end, unsigned int extra_bits,
+                                     struct extent_state **cached_state)
 {
        return set_extent_bit(tree, start, end,
-                             EXTENT_DELALLOC | EXTENT_UPTODATE,
+                             EXTENT_DELALLOC | EXTENT_UPTODATE | extra_bits,
                              NULL, cached_state, GFP_NOFS);
 }
 
index 47e6ebad78c36c07a835fd458e8d6c5d8e12d741..2de7b4f4ca3c7cb1d05d373100957dc0424fc83d 100644 (file)
@@ -538,14 +538,34 @@ int btrfs_dirty_pages(struct inode *inode, struct page **pages,
        u64 end_of_last_block;
        u64 end_pos = pos + write_bytes;
        loff_t isize = i_size_read(inode);
+       unsigned int extra_bits = 0;
 
        start_pos = pos & ~((u64) fs_info->sectorsize - 1);
        num_bytes = round_up(write_bytes + pos - start_pos,
                             fs_info->sectorsize);
 
        end_of_last_block = start_pos + num_bytes - 1;
+
+       if (!btrfs_is_free_space_inode(BTRFS_I(inode))) {
+               if (start_pos >= isize &&
+                   !(BTRFS_I(inode)->flags & BTRFS_INODE_PREALLOC)) {
+                       /*
+                        * There can't be any extents following eof in this case
+                        * so just set the delalloc new bit for the range
+                        * directly.
+                        */
+                       extra_bits |= EXTENT_DELALLOC_NEW;
+               } else {
+                       err = btrfs_find_new_delalloc_bytes(BTRFS_I(inode),
+                                                           start_pos,
+                                                           num_bytes, cached);
+                       if (err)
+                               return err;
+               }
+       }
+
        err = btrfs_set_extent_delalloc(inode, start_pos, end_of_last_block,
-                                       cached, 0);
+                                       extra_bits, cached, 0);
        if (err)
                return err;
 
@@ -1473,10 +1493,8 @@ lock_and_cleanup_extent_if_need(struct btrfs_inode *inode, struct page **pages,
                + round_up(pos + write_bytes - start_pos,
                           fs_info->sectorsize) - 1;
 
-       if (start_pos < inode->vfs_inode.i_size ||
-           (inode->flags & BTRFS_INODE_PREALLOC)) {
+       if (start_pos < inode->vfs_inode.i_size) {
                struct btrfs_ordered_extent *ordered;
-               unsigned int clear_bits;
 
                lock_extent_bits(&inode->io_tree, start_pos, last_pos,
                                cached_state);
@@ -1498,19 +1516,10 @@ lock_and_cleanup_extent_if_need(struct btrfs_inode *inode, struct page **pages,
                }
                if (ordered)
                        btrfs_put_ordered_extent(ordered);
-               ret = btrfs_find_new_delalloc_bytes(inode, start_pos,
-                                                   last_pos - start_pos + 1,
-                                                   cached_state);
-               clear_bits = EXTENT_DIRTY | EXTENT_DELALLOC |
-                       EXTENT_DO_ACCOUNTING | EXTENT_DEFRAG;
-               if (ret)
-                       clear_bits |= EXTENT_DELALLOC_NEW | EXTENT_LOCKED;
-               clear_extent_bit(&inode->io_tree, start_pos,
-                                last_pos, clear_bits,
-                                (clear_bits & EXTENT_LOCKED) ? 1 : 0,
-                                0, cached_state, GFP_NOFS);
-               if (ret)
-                       return ret;
+               clear_extent_bit(&inode->io_tree, start_pos, last_pos,
+                                EXTENT_DIRTY | EXTENT_DELALLOC |
+                                EXTENT_DO_ACCOUNTING | EXTENT_DEFRAG,
+                                0, 0, cached_state, GFP_NOFS);
                *lockstart = start_pos;
                *lockend = last_pos;
                ret = 1;
index 1131db7a0b2871352ac595fd422c987dee27f79a..993061f83067a9a65c5a58908908f5948423f11a 100644 (file)
@@ -2032,11 +2032,12 @@ static noinline int add_pending_csums(struct btrfs_trans_handle *trans,
 }
 
 int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
+                             unsigned int extra_bits,
                              struct extent_state **cached_state, int dedupe)
 {
        WARN_ON((end & (PAGE_SIZE - 1)) == 0);
        return set_extent_delalloc(&BTRFS_I(inode)->io_tree, start, end,
-                                  cached_state);
+                                  extra_bits, cached_state);
 }
 
 /* see btrfs_writepage_start_hook for details on why this is required */
@@ -2097,7 +2098,7 @@ again:
                goto out;
         }
 
-       btrfs_set_extent_delalloc(inode, page_start, page_end, &cached_state,
+       btrfs_set_extent_delalloc(inode, page_start, page_end, 0, &cached_state,
                                  0);
        ClearPageChecked(page);
        set_page_dirty(page);
@@ -4797,7 +4798,7 @@ again:
                          EXTENT_DO_ACCOUNTING | EXTENT_DEFRAG,
                          0, 0, &cached_state, GFP_NOFS);
 
-       ret = btrfs_set_extent_delalloc(inode, block_start, block_end,
+       ret = btrfs_set_extent_delalloc(inode, block_start, block_end, 0,
                                        &cached_state, 0);
        if (ret) {
                unlock_extent_cached(io_tree, block_start, block_end,
@@ -9163,7 +9164,7 @@ again:
                          EXTENT_DO_ACCOUNTING | EXTENT_DEFRAG,
                          0, 0, &cached_state, GFP_NOFS);
 
-       ret = btrfs_set_extent_delalloc(inode, page_start, end,
+       ret = btrfs_set_extent_delalloc(inode, page_start, end, 0,
                                        &cached_state, 0);
        if (ret) {
                unlock_extent_cached(io_tree, page_start, page_end,
index 4cf2eb67eba6ceceeae466c69bf4b5dc188a7339..f0c3f00e97cbe76e1fa8484efc8933842856c8d5 100644 (file)
@@ -3268,7 +3268,8 @@ static int relocate_file_extent_cluster(struct inode *inode,
                        nr++;
                }
 
-               btrfs_set_extent_delalloc(inode, page_start, page_end, NULL, 0);
+               btrfs_set_extent_delalloc(inode, page_start, page_end, 0, NULL,
+                                         0);
                set_page_dirty(page);
 
                unlock_extent(&BTRFS_I(inode)->io_tree,
index d06b1c931d05b8ef2b06c8dbc26d7657c92010c7..2e7f64a3b22b7d55d0bc6abfccd09171ef943351 100644 (file)
@@ -114,7 +114,7 @@ static int test_find_delalloc(u32 sectorsize)
         * |--- delalloc ---|
         * |---  search  ---|
         */
-       set_extent_delalloc(&tmp, 0, sectorsize - 1, NULL);
+       set_extent_delalloc(&tmp, 0, sectorsize - 1, 0, NULL);
        start = 0;
        end = 0;
        found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
@@ -145,7 +145,7 @@ static int test_find_delalloc(u32 sectorsize)
                test_msg("Couldn't find the locked page\n");
                goto out_bits;
        }
-       set_extent_delalloc(&tmp, sectorsize, max_bytes - 1, NULL);
+       set_extent_delalloc(&tmp, sectorsize, max_bytes - 1, 0, NULL);
        start = test_start;
        end = 0;
        found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
@@ -200,7 +200,7 @@ static int test_find_delalloc(u32 sectorsize)
         *
         * We are re-using our test_start from above since it works out well.
         */
-       set_extent_delalloc(&tmp, max_bytes, total_dirty - 1, NULL);
+       set_extent_delalloc(&tmp, max_bytes, total_dirty - 1, 0, NULL);
        start = test_start;
        end = 0;
        found = find_lock_delalloc_range(inode, &tmp, locked_page, &start,
index f797642c013dadc24f5391fd148bcdc6d320c563..30affb60da514848ef8fb7621a48e629e893feb3 100644 (file)
@@ -968,7 +968,7 @@ static int test_extent_accounting(u32 sectorsize, u32 nodesize)
        btrfs_test_inode_set_ops(inode);
 
        /* [BTRFS_MAX_EXTENT_SIZE] */
-       ret = btrfs_set_extent_delalloc(inode, 0, BTRFS_MAX_EXTENT_SIZE - 1,
+       ret = btrfs_set_extent_delalloc(inode, 0, BTRFS_MAX_EXTENT_SIZE - 1, 0,
                                        NULL, 0);
        if (ret) {
                test_msg("btrfs_set_extent_delalloc returned %d\n", ret);
@@ -984,7 +984,7 @@ static int test_extent_accounting(u32 sectorsize, u32 nodesize)
        /* [BTRFS_MAX_EXTENT_SIZE][sectorsize] */
        ret = btrfs_set_extent_delalloc(inode, BTRFS_MAX_EXTENT_SIZE,
                                        BTRFS_MAX_EXTENT_SIZE + sectorsize - 1,
-                                       NULL, 0);
+                                       0, NULL, 0);
        if (ret) {
                test_msg("btrfs_set_extent_delalloc returned %d\n", ret);
                goto out;
@@ -1018,7 +1018,7 @@ static int test_extent_accounting(u32 sectorsize, u32 nodesize)
        ret = btrfs_set_extent_delalloc(inode, BTRFS_MAX_EXTENT_SIZE >> 1,
                                        (BTRFS_MAX_EXTENT_SIZE >> 1)
                                        + sectorsize - 1,
-                                       NULL, 0);
+                                       0, NULL, 0);
        if (ret) {
                test_msg("btrfs_set_extent_delalloc returned %d\n", ret);
                goto out;
@@ -1036,7 +1036,7 @@ static int test_extent_accounting(u32 sectorsize, u32 nodesize)
        ret = btrfs_set_extent_delalloc(inode,
                        BTRFS_MAX_EXTENT_SIZE + 2 * sectorsize,
                        (BTRFS_MAX_EXTENT_SIZE << 1) + 3 * sectorsize - 1,
-                       NULL, 0);
+                       0, NULL, 0);
        if (ret) {
                test_msg("btrfs_set_extent_delalloc returned %d\n", ret);
                goto out;
@@ -1053,7 +1053,7 @@ static int test_extent_accounting(u32 sectorsize, u32 nodesize)
        */
        ret = btrfs_set_extent_delalloc(inode,
                        BTRFS_MAX_EXTENT_SIZE + sectorsize,
-                       BTRFS_MAX_EXTENT_SIZE + 2 * sectorsize - 1, NULL, 0);
+                       BTRFS_MAX_EXTENT_SIZE + 2 * sectorsize - 1, 0, NULL, 0);
        if (ret) {
                test_msg("btrfs_set_extent_delalloc returned %d\n", ret);
                goto out;
@@ -1089,7 +1089,7 @@ static int test_extent_accounting(u32 sectorsize, u32 nodesize)
         */
        ret = btrfs_set_extent_delalloc(inode,
                        BTRFS_MAX_EXTENT_SIZE + sectorsize,
-                       BTRFS_MAX_EXTENT_SIZE + 2 * sectorsize - 1, NULL, 0);
+                       BTRFS_MAX_EXTENT_SIZE + 2 * sectorsize - 1, 0, NULL, 0);
        if (ret) {
                test_msg("btrfs_set_extent_delalloc returned %d\n", ret);
                goto out;