On Fri, Mar 29, 2024 at 08:25:55AM -0400, Sasha Levin wrote:
> From: Filipe Manana <fdmanana@xxxxxxxx>
>
> [ Upstream commit e383e158ed1b6abc2d2d3e6736d77a46393f80fa ]
>
> When logging an inode and we require to copy items from subvolume leaves
> to the log tree, we clone each subvolume leaf and than use that clone to
> copy items to the log tree. This is required to avoid possible deadlocks
> as stated in commit 796787c978ef ("btrfs: do not modify log tree while
> holding a leaf from fs tree locked").
>
> The cloning requires allocating an extent buffer (struct extent_buffer)
> and then allocating pages (folios) to attach to the extent buffer. This
> may be slow in case we are under memory pressure, and since we are doing
> the cloning while holding a read lock on a subvolume leaf, it means we
> can be blocking other operations on that leaf for significant periods of
> time, which can increase latency on operations like creating other files,
> renaming files, etc. Similarly because we're under a log transaction, we
> may also cause extra delay on other tasks doing an fsync, because syncing
> the log requires waiting for tasks that joined a log transaction to exit
> the transaction.
>
> So to improve this, for any inode logging operation that needs to copy
> items from a subvolume leaf ("full sync" or "copy everything" bit set
> in the inode), preallocate a dummy extent buffer before locking any
> extent buffer from the subvolume tree, and even before joining a log
> transaction, add it to the log context and then use it when we need to
> copy items from a subvolume leaf to the log tree. This avoids making
> other operations get extra latency when waiting to lock a subvolume
> leaf that is used during inode logging and we are under heavy memory
> pressure.
>
> The following test script with bonnie++ was used to test this:
>
> $ cat test.sh
> #!/bin/bash
>
> DEV=/dev/sdh
> MNT=/mnt/sdh
> MOUNT_OPTIONS="-o ssd"
>
> MEMTOTAL_BYTES=`free -b | grep Mem: | awk '{ print $2 }'`
> NR_DIRECTORIES=20
> NR_FILES=20480
> DATASET_SIZE=$((MEMTOTAL_BYTES * 2 / 1048576))
> DIRECTORY_SIZE=$((MEMTOTAL_BYTES * 2 / NR_FILES))
> NR_FILES=$((NR_FILES / 1024))
>
> echo "performance" | \
> tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
>
> umount $DEV &> /dev/null
> mkfs.btrfs -f $MKFS_OPTIONS $DEV
> mount $MOUNT_OPTIONS $DEV $MNT
>
> bonnie++ -u root -d $MNT \
> -n $NR_FILES:$DIRECTORY_SIZE:$DIRECTORY_SIZE:$NR_DIRECTORIES \
> -r 0 -s $DATASET_SIZE -b
>
> umount $MNT
>
> The results of this test on a 8G VM running a non-debug kernel (Debian's
> default kernel config), were the following.
>
> Before this change:
>
> Version 2.00a ------Sequential Output------ --Sequential Input- --Random-
> -Per Chr- --Block-- -Rewrite- -Per Chr- --Block-- --Seeks--
> Name:Size etc /sec %CP /sec %CP /sec %CP /sec %CP /sec %CP /sec %CP
> debian0 7501M 376k 99 1.4g 96 117m 14 1510k 99 2.5g 95 +++++ +++
> Latency 35068us 24976us 2944ms 30725us 71770us 26152us
> Version 2.00a ------Sequential Create------ --------Random Create--------
> debian0 -Create-- --Read--- -Delete-- -Create-- --Read--- -Delete--
> files:max:min /sec %CP /sec %CP /sec %CP /sec %CP /sec %CP /sec %CP
> 20:384100:384100/20 20480 32 20480 58 20480 48 20480 39 20480 56 20480 61
> Latency 411ms 11914us 119ms 617ms 10296us 110ms
>
> After this change:
>
> Version 2.00a ------Sequential Output------ --Sequential Input- --Random-
> -Per Chr- --Block-- -Rewrite- -Per Chr- --Block-- --Seeks--
> Name:Size etc /sec %CP /sec %CP /sec %CP /sec %CP /sec %CP /sec %CP
> debian0 7501M 375k 99 1.4g 97 117m 14 1546k 99 2.3g 98 +++++ +++
> Latency 35975us 20945us 2144ms 10297us 2217us 6004us
> Version 2.00a ------Sequential Create------ --------Random Create--------
> debian0 -Create-- --Read--- -Delete-- -Create-- --Read--- -Delete--
> files:max:min /sec %CP /sec %CP /sec %CP /sec %CP /sec %CP /sec %CP
> 20:384100:384100/20 20480 35 20480 58 20480 48 20480 40 20480 57 20480 59
> Latency 320ms 11237us 77779us 518ms 6470us 86389us
>
> Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> Reviewed-by: David Sterba <dsterba@xxxxxxxx>
> Signed-off-by: David Sterba <dsterba@xxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
This is a performance improvement, how does this qualify for stable? I
read only about notable perfromance fixes but this is not one.
