Re: [PATCH 0/4] btrfs: make autodefrag to defrag and only defrag small write ranges

[Qu Wenruo <wqu@suse.com>]

On 2022/2/13 15:42, Qu Wenruo wrote:
> When a small write reaches disk, btrfs will mark the inode for
> autodefrag, and record the transid of the inode for autodefrag.
> 
> Then autodefrag uses the transid to only scan newer file extents.
> 
> However this transid based scanning scheme has a hole, the small write
> size threshold triggering autodefrag is not the same extent size
> threshold for autodefrag.
> 
> For the following write sequence on an non-compressed inode:
> 
>   pwrite 0 4k
>   sync
>   pwrite 4k 128k
>   sync
>   pwrite 132k 128k
>   sync.
> 
> The first 4K is indeed a small write (<64K), but the later two 128K ones
> are definite not (>64K).
> 
> Hoever autodefrag will try to defrag all three writes, as the
> extent_threshold used for autodefrag is fixed 256K.
> 
> This extra scanning on extents which didn't trigger autodefrag can cause
> extra IO.
> 
> This patchset will try to address the problem by:
> 
> - Remove the inode_defrag re-queue behavior
>    Now we just scan one file til its end (while keep the
>    max_sectors_to_defrag limit, and frequently check the root refs, and
>    remount situation to exit).
> 
>    This also saves several bytes from inode_defrag structure.
> 
>    This is done in the 3rd patch.
> 
> - Save @small_write value into inode_defrag and use it as autodefrag
>    extent threshold
>    Now there is no gap for autodefrag and small_write.
> 
>    This is done in the 4th patch.
> 
> The remaining patches are:
> 
> - Removing one dead parameter
> 
> - Add extra trace events for autodefrag
>    So end users will no longer need to re-compile kernel modules, and
>    use trace events to provide debug info on the autodefrag/defrag ioctl.
> 
> Unfortunately I don't have a good benchmark setup for the patchset yet,
> but unlike previous RFC version, this one brings very little extra
> resource usage, and is just changing the extent_threshold for
> autodefrag.

Got a small benchmark result for it.

Using the following fio job:

  [torrent]
  filename=torrent-test
  rw=randwrite
  ioengine=sync
  size=4g
  randseed=123456
  allrandrepeat=1
  fallocate=none

And the VM only has 2G ram.

This should really be the worst case scenario.

Then the full benchmark includes:

start_trace()
{
	echo 0 > $tracedir/tracing_on
	echo > $tracedir/trace
	#echo > $tracedir/trace_options
	echo > $tracedir/set_event
	echo "btrfs:defrag_file_end" >> $tracedir/set_event
	echo 1 > $tracedir/tracing_on
}

end_trace()
{
	cp $tracedir/trace /home/adam
	echo 0 > $tracedir/tracing_on
}

	mkfs.btrfs -f $dev

	start_trace
	mount $dev $mnt -o autodefrag
	cd $mnt
	fio /home/adam/torrent.fio
	cd
	umount $mnt
	end_trace

With all defragged sectors accounted, before the last two patches:

Total sectors defragged		= 6846831
Total defrag_file() calls	= 6701

After the last two patches:

Total sectors defragged		= 3466851
Total defrag_file() calls	= 3396

Which shows an obvious drop in the sectors marked for autodefrag.

Thanks,
Qu




> 
> Changelog:
> RFC->v1:
> - Add ftrace events for defrag
> 
> - Add a new patch to change how we run defrag inodes
>    Instead of saving previous location and re-queue, just run it in one
>    run.
>    Previously btrfs_run_defrag_inodse() will always exhaust the existing
>    inode_defrag anyway, the change should not bring much difference.
> 
> - Change autodefrag extent_thresh to close the gap, other than using
>    another extent io tree
>    Now it uses less resource, keep the critical section small, while
>    can almost reach the same objective.
> 
> Qu Wenruo (4):
>    btrfs: remove unused parameter for btrfs_add_inode_defrag()
>    btrfs: add trace events for defrag
>    btrfs: autodefrag: only scan one inode once
>    btrfs: close the gap between inode_should_defrag() and autodefrag
>      extent size threshold
> 
>   fs/btrfs/ctree.h             |   3 +-
>   fs/btrfs/file.c              | 165 +++++++++++++++--------------------
>   fs/btrfs/inode.c             |   4 +-
>   fs/btrfs/ioctl.c             |   4 +
>   include/trace/events/btrfs.h | 127 +++++++++++++++++++++++++++
>   5 files changed, 206 insertions(+), 97 deletions(-)
>