Skip to content

Commit

Permalink
btrfs: don't set the full sync flag when truncation does not touch ex…
Browse files Browse the repository at this point in the history
…tents

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>
  • Loading branch information
Filipe Manana authored and David Sterba committed Jun 21, 2021
1 parent 4f7e673 commit 0d7d316
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 15 deletions.
2 changes: 1 addition & 1 deletion fs/btrfs/ctree.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion fs/btrfs/free-space-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
41 changes: 30 additions & 11 deletions fs/btrfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -4482,6 +4482,11 @@ static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
* @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
Expand All @@ -4495,7 +4500,8 @@ static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
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;
Expand Down Expand Up @@ -4641,6 +4647,9 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions fs/btrfs/tree-log.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 0d7d316

Please sign in to comment.