From 0d7d316597c00fbc13fffadaab27a448d5a6a60f Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Mon, 24 May 2021 11:35:55 +0100 Subject: btrfs: don't set the full sync flag when truncation does not touch extents 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 Signed-off-by: Filipe Manana Signed-off-by: David Sterba --- fs/btrfs/tree-log.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs/btrfs/tree-log.c') diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index dbcf8bb2f3b9..c6d4aeede159 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -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; } -- cgit v1.2.3 From 1aeb6b563aea18cd55c73cf666d1d3245a00f08c Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 7 Jul 2020 18:38:05 +0200 Subject: btrfs: clear log tree recovering status if starting transaction fails When a log recovery is in progress, lots of operations have to take that into account, so we keep this status per tree during the operation. Long time ago error handling revamp patch 79787eaab461 ("btrfs: replace many BUG_ONs with proper error handling") removed clearing of the status in an error branch. Add it back as was intended in e02119d5a7b4 ("Btrfs: Add a write ahead tree log to optimize synchronous operations"). There are probably no visible effects, log replay is done only during mount and if it fails all structures are cleared so the stale status won't be kept. Fixes: 79787eaab461 ("btrfs: replace many BUG_ONs with proper error handling") Reviewed-by: Qu Wenruo Reviewed-by: Anand Jain Signed-off-by: David Sterba --- fs/btrfs/tree-log.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/btrfs/tree-log.c') diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index c6d4aeede159..5c1d58706fa9 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -6372,6 +6372,7 @@ next: error: if (wc.trans) btrfs_end_transaction(wc.trans); + clear_bit(BTRFS_FS_LOG_RECOVERING, &fs_info->flags); btrfs_free_path(path); return ret; } -- cgit v1.2.3 From b590b839720cf4fa46798ee6e950ed7369f52a15 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Fri, 28 May 2021 11:37:32 +0100 Subject: btrfs: avoid unnecessary logging of xattrs during fast fsyncs When logging an inode we always log all its xattrs, so that we are able to figure out which ones should be deleted during log replay. However this is unnecessary when we are doing a fast fsync and no xattrs were added, changed or deleted since the last time we logged the inode in the current transaction. So skip the logging of xattrs when the inode was previously logged in the current transaction and no xattrs were added, changed or deleted. If any changes to xattrs happened, than the inode has BTRFS_INODE_COPY_EVERYTHING set in its runtime flags and the xattrs get logged. This saves time on scanning for xattrs, allocating memory, COWing log tree extent buffers and adding more lock contention on the extent buffers when there are multiple tasks logging in parallel. The use of xattrs is common when using ACLs, some applications, or when using security modules like SELinux where every inode gets a security xattr added to it. The following test script, using fio, was used on a box with 12 cores, 64G of RAM, a NVMe device and the default non-debug kernel config from Debian. It uses 8 concurrent jobs each writing in blocks of 64K to its own 4G file, each file with a single xattr of 50 bytes (about the same size for an ACL or SELinux xattr), doing random buffered writes with an fsync after each write. $ cat test.sh #!/bin/bash DEV=/dev/nvme0n1 MNT=/mnt/test MOUNT_OPTIONS="-o ssd" MKFS_OPTIONS="-d single -m single" NUM_JOBS=8 FILE_SIZE=4G cat < /tmp/fio-job.ini [writers] rw=randwrite fsync=1 fallocate=none group_reporting=1 direct=0 bs=64K ioengine=sync size=$FILE_SIZE directory=$MNT numjobs=$NUM_JOBS EOF echo "performance" | \ tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor mkfs.btrfs -f $MKFS_OPTIONS $DEV > /dev/null mount $MOUNT_OPTIONS $DEV $MNT echo "Creating files before fio runs, each with 1 xattr of 50 bytes" for ((i = 0; i < $NUM_JOBS; i++)); do path="$MNT/writers.$i.0" truncate -s $FILE_SIZE $path setfattr -n user.xa1 -v $(printf '%0.sX' $(seq 50)) $path done fio /tmp/fio-job.ini umount $MNT fio output before this change: WRITE: bw=120MiB/s (126MB/s), 120MiB/s-120MiB/s (126MB/s-126MB/s), io=32.0GiB (34.4GB), run=272145-272145msec fio output after this change: WRITE: bw=142MiB/s (149MB/s), 142MiB/s-142MiB/s (149MB/s-149MB/s), io=32.0GiB (34.4GB), run=230408-230408msec +16.8% throughput, -16.6% runtime Signed-off-by: Filipe Manana Signed-off-by: David Sterba --- fs/btrfs/tree-log.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'fs/btrfs/tree-log.c') diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index 5c1d58706fa9..cab451d19547 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -5467,13 +5467,23 @@ log_extents: btrfs_release_path(dst_path); if (need_log_inode_item) { err = log_inode_item(trans, log, dst_path, inode); - if (!err && !xattrs_logged) { + if (err) + goto out_unlock; + /* + * If we are doing a fast fsync and the inode was logged before + * in this transaction, we don't need to log the xattrs because + * they were logged before. If xattrs were added, changed or + * deleted since the last time we logged the inode, then we have + * already logged them because the inode had the runtime flag + * BTRFS_INODE_COPY_EVERYTHING set. + */ + if (!xattrs_logged && inode->logged_trans < trans->transid) { err = btrfs_log_all_xattrs(trans, root, inode, path, dst_path); + if (err) + goto out_unlock; btrfs_release_path(path); } - if (err) - goto out_unlock; } if (fast_search) { ret = btrfs_log_changed_extents(trans, root, inode, dst_path, -- cgit v1.2.3