btrfs: don't set the full sync flag when truncation does not touch extents
authorFilipe Manana <fdmanana@suse.com>
Mon, 24 May 2021 10:35:55 +0000 (11:35 +0100)
committerDavid Sterba <dsterba@suse.com>
Mon, 21 Jun 2021 13:19:05 +0000 (15:19 +0200)
At btrfs_truncate() where we truncate the inode either to the same size
or to a smaller size, we always set the full sync flag on the inode.

This is needed in case the truncation drops or trims any file extent items
that start beyond or cross the new inode size, so that the next fsync
drops all inode items from the log and scans again the fs/subvolume tree
to find all items that must be logged.

However if the truncation does not drop or trims any file extent items, we
do not need to set the full sync flag and force the next fsync to use the
slow code path. So do not set the full sync flag in such cases.

One use case where it is frequent to do truncations that do not change
the inode size and do not drop any extents (no prealloc extents beyond
i_size) is when running Microsoft's SQL Server inside a Docker container.
One example workload is the one Philipp Fent reported recently, in the
thread with a link below. In this workload a large number of fsyncs are
preceded by such truncate operations.

After this change I constantly get the runtime for that workload from
Philipp to be reduced by about -12%, for example from 184 seconds down
to 162 seconds.

Link: https://lore.kernel.org/linux-btrfs/93c4600e-5263-5cba-adf0-6f47526e7561@in.tum.de/
Tested-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/ctree.h
fs/btrfs/free-space-cache.c
fs/btrfs/inode.c
fs/btrfs/tree-log.c

index 938d8eb..d78cb2d 100644 (file)
@@ -3125,7 +3125,7 @@ int btrfs_truncate_block(struct btrfs_inode *inode, loff_t from, loff_t len,
 int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
                               struct btrfs_root *root,
                               struct btrfs_inode *inode, u64 new_size,
-                              u32 min_type);
+                              u32 min_type, u64 *extents_found);
 
 int btrfs_start_delalloc_snapshot(struct btrfs_root *root, bool in_reclaim_context);
 int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, long nr,
index 4806295..2131ae5 100644 (file)
@@ -327,7 +327,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_trans_handle *trans,
         * need to check for -EAGAIN.
         */
        ret = btrfs_truncate_inode_items(trans, root, BTRFS_I(inode),
-                                        0, BTRFS_EXTENT_DATA_KEY);
+                                        0, BTRFS_EXTENT_DATA_KEY, NULL);
        if (ret)
                goto fail;
 
index 30643ef..3cd5286 100644 (file)
@@ -4482,6 +4482,11 @@ out:
  * @min_type:          The minimum key type to remove. All keys with a type
  *                     greater than this value are removed and all keys with
  *                     this type are removed only if their offset is >= @new_size.
+ * @extents_found:     Output parameter that will contain the number of file
+ *                     extent items that were removed or adjusted to the new
+ *                     inode i_size. The caller is responsible for initializing
+ *                     the counter. Also, it can be NULL if the caller does not
+ *                     need this counter.
  *
  * Remove all keys associated with the inode from the given root that have a key
  * with a type greater than or equals to @min_type. When @min_type has a value of
@@ -4495,7 +4500,8 @@ out:
 int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
                               struct btrfs_root *root,
                               struct btrfs_inode *inode,
-                              u64 new_size, u32 min_type)
+                              u64 new_size, u32 min_type,
+                              u64 *extents_found)
 {
        struct btrfs_fs_info *fs_info = root->fs_info;
        struct btrfs_path *path;
@@ -4641,6 +4647,9 @@ search_again:
                if (found_type != BTRFS_EXTENT_DATA_KEY)
                        goto delete;
 
+               if (extents_found != NULL)
+                       (*extents_found)++;
+
                if (extent_type != BTRFS_FILE_EXTENT_INLINE) {
                        u64 num_dec;
 
@@ -5473,7 +5482,7 @@ void btrfs_evict_inode(struct inode *inode)
                trans->block_rsv = rsv;
 
                ret = btrfs_truncate_inode_items(trans, root, BTRFS_I(inode),
-                                                0, 0);
+                                                0, 0, NULL);
                trans->block_rsv = &fs_info->trans_block_rsv;
                btrfs_end_transaction(trans);
                btrfs_btree_balance_dirty(fs_info);
@@ -8677,6 +8686,7 @@ static int btrfs_truncate(struct inode *inode, bool skip_writeback)
        struct btrfs_trans_handle *trans;
        u64 mask = fs_info->sectorsize - 1;
        u64 min_size = btrfs_calc_metadata_size(fs_info, 1);
+       u64 extents_found = 0;
 
        if (!skip_writeback) {
                ret = btrfs_wait_ordered_range(inode, inode->i_size & (~mask),
@@ -8734,20 +8744,13 @@ static int btrfs_truncate(struct inode *inode, bool skip_writeback)
                                      min_size, false);
        BUG_ON(ret);
 
-       /*
-        * So if we truncate and then write and fsync we normally would just
-        * write the extents that changed, which is a problem if we need to
-        * first truncate that entire inode.  So set this flag so we write out
-        * all of the extents in the inode to the sync log so we're completely
-        * safe.
-        */
-       set_bit(BTRFS_INODE_NEEDS_FULL_SYNC, &BTRFS_I(inode)->runtime_flags);
        trans->block_rsv = rsv;
 
        while (1) {
                ret = btrfs_truncate_inode_items(trans, root, BTRFS_I(inode),
                                                 inode->i_size,
-                                                BTRFS_EXTENT_DATA_KEY);
+                                                BTRFS_EXTENT_DATA_KEY,
+                                                &extents_found);
                trans->block_rsv = &fs_info->trans_block_rsv;
                if (ret != -ENOSPC && ret != -EAGAIN)
                        break;
@@ -8809,6 +8812,22 @@ static int btrfs_truncate(struct inode *inode, bool skip_writeback)
        }
 out:
        btrfs_free_block_rsv(fs_info, rsv);
+       /*
+        * So if we truncate and then write and fsync we normally would just
+        * write the extents that changed, which is a problem if we need to
+        * first truncate that entire inode.  So set this flag so we write out
+        * all of the extents in the inode to the sync log so we're completely
+        * safe.
+        *
+        * If no extents were dropped or trimmed we don't need to force the next
+        * fsync to truncate all the inode's items from the log and re-log them
+        * all. This means the truncate operation did not change the file size,
+        * or changed it to a smaller size but there was only an implicit hole
+        * between the old i_size and the new i_size, and there were no prealloc
+        * extents beyond i_size to drop.
+        */
+       if (extents_found > 0)
+               set_bit(BTRFS_INODE_NEEDS_FULL_SYNC, &BTRFS_I(inode)->runtime_flags);
 
        return ret;
 }
index dbcf8bb..c6d4aee 100644 (file)
@@ -4468,7 +4468,8 @@ static int btrfs_log_prealloc_extents(struct btrfs_trans_handle *trans,
                                ret = btrfs_truncate_inode_items(trans,
                                                         root->log_root,
                                                         inode, truncate_offset,
-                                                        BTRFS_EXTENT_DATA_KEY);
+                                                        BTRFS_EXTENT_DATA_KEY,
+                                                        NULL);
                        } while (ret == -EAGAIN);
                        if (ret)
                                goto out;
@@ -5416,7 +5417,7 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans,
                                          &inode->runtime_flags);
                                while(1) {
                                        ret = btrfs_truncate_inode_items(trans,
-                                               log, inode, 0, 0);
+                                               log, inode, 0, 0, NULL);
                                        if (ret != -EAGAIN)
                                                break;
                                }