Re: [PATCH v3 10/12] btrfs: Optimize unallocated chunks discard

[David Sterba <dsterba@suse.cz>]

On Mon, Mar 25, 2019 at 02:31:30PM +0200, Nikolay Borisov wrote:
> Currently unallocated chunks are always trimmed. For example
> 2 consecutive trims on large storage would trim freespace twice
> irrespective of whether the space was actually allocated or not between
> those trims.
> 
> Optimise this behavior by exploiting the newly introduced alloc_state
> tree of btrfs_device. A new CHUNK_TRIMMED bit is used to mark
> those unallocated chunks which have been trimmed and have not been
> allocated afterwards. On chunk allocation the respective underlying devices'
> physical space will have its CHUNK_TRIMMED flag cleared. This avoids
> submitting discards for space which hasn't been changed since the last
> time discard was issued.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/extent-tree.c | 57 +++++++++++++++++++++++++++++++++++++++++-
>  fs/btrfs/extent_io.h   |  8 +++++-
>  fs/btrfs/extent_map.c  |  4 ++-
>  3 files changed, 66 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 574c73e0a7c0..503d68ba3f6a 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -11249,6 +11249,54 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
>  	return unpin_extent_range(fs_info, start, end, false);
>  }
>  
> +static bool should_skip_trim(struct btrfs_device *device, u64 *start, u64 *len)
> +{
> +	u64 trimmed_start = 0, trimmed_end = 0;
> +	u64 end = *start + *len - 1;
> +
> +	if (!find_first_extent_bit(&device->alloc_state, *start, &trimmed_start,
> +				   &trimmed_end, CHUNK_TRIMMED, NULL)) {
> +		u64 trimmed_len = trimmed_end - trimmed_start + 1;
> +
> +		if (*start < trimmed_start) {
> +			if (in_range(end, trimmed_start, trimmed_len) ||
> +			    end > trimmed_end) {
> +				/*
> +				 * start|------|end
> +				 *      ts|--|trimmed_len
> +				 *      OR
> +				 * start|-----|end
> +				 *      ts|-----|trimmed_len
> +				 */
> +				*len = trimmed_start - *start;
> +				return false;
> +			} else if (end < trimmed_start) {
> +				/*
> +				 * start|------|end
> +				 *             ts|--|trimmed_len
> +				 */
> +				return false;
> +			}
> +		} else if (in_range(*start, trimmed_start, trimmed_len)) {
> +			if (in_range(end, trimmed_start, trimmed_len)) {
> +				/*
> +				 * start|------|end
> +				 *  ts|----------|trimmed_len
> +				 */
> +				return true;
> +			} else {
> +				/*
> +				 * start|-----------|end
> +				 *  ts|----------|trimmed_len
> +				 */
> +				*start = trimmed_end + 1;
> +				*len = end - *start + 1;
> +				return false;
> +			}
> +		}
> +	}
> +	return false;
> +}
>  /*
>   * It used to be that old block groups would be left around forever.
>   * Iterating over them would be enough to trim unused space.  Since we
> @@ -11319,7 +11367,14 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>  		start = max(range->start, start);
>  		len = min(range->len, len);
>  
> -		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
> +		if (!should_skip_trim(device, &start, &len)) {
> +			ret = btrfs_issue_discard(device->bdev, start, len,
> +						  &bytes);
> +			if (!ret)
> +				set_extent_bits(&device->alloc_state, start,
> +						start + bytes - 1,
> +						CHUNK_TRIMMED);
> +		}
>  		mutex_unlock(&fs_info->chunk_mutex);
>  
>  		if (ret)
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index 4bcc203b5431..9dd5190d9dd8 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -28,8 +28,14 @@
>  #define EXTENT_CTLBITS		(EXTENT_DO_ACCOUNTING)
>  
>  
> -/* Redefined bits above which are used only in the device allocation tree */
> +/*
> + * Redefined bits above which are used only in the device allocation tree,
> + * shouldn't be using EXTENT_IOBITS(EXTENT_LOCKED/EXTENT_WRITEBACK) /
> + * EXTENT_BOUNDARY / EXTENT_CLEAR_META_RESV / EXTENT_CLEAR_DATA_RESV because

Fixup after recent changes in misc-net: EXTENT_IOBITS and EXTENT_WRITEBACK
are gone, comment updated.

> + * they have special meaning to the bit manipulation functions
> + */
>  #define CHUNK_ALLOCATED EXTENT_DIRTY
> +#define CHUNK_TRIMMED   EXTENT_DEFRAG
>  
>  /*
>   * flags for bio submission. The high bits indicate the compression
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 0820f6fcf3a6..9e8c0904f623 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -389,8 +389,10 @@ int add_extent_mapping(struct extent_map_tree *tree,
>  		goto out;
>  
>  	setup_extent_mapping(tree, em, modified);
> -	if (test_bit(EXTENT_FLAG_FS_MAPPING, &em->flags))
> +	if (test_bit(EXTENT_FLAG_FS_MAPPING, &em->flags)) {
>  		extent_map_device_set_bits(em, CHUNK_ALLOCATED);
> +		extent_map_device_clear_bits(em, CHUNK_TRIMMED);
> +	}
>  out:
>  	return ret;
>  }
> -- 
> 2.17.1