Re: [PATCH v3 07/12] btrfs: replace pending/pinned chunks lists with io tree

[David Sterba <dsterba@suse.cz>]

On Mon, Mar 25, 2019 at 02:31:27PM +0200, Nikolay Borisov wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> The pending chunks list contains chunks that are allocated in the
> current transaction but haven't been created yet. The pinned chunks
> list contains chunks that are being released in the current transaction.
> Both describe chunks that are not reflected on disk as in use but are
> unavailable just the same.
> 
> The pending chunks list is anchored by the transaction handle, which
> means that we need to hold a reference to a transaction when working
> with the list.
> 
> We use these lists to ensure that we don't end up discarding chunks
> that are allocated or released in the current transaction.  What we r
> 
> The way we use them is by iterating over both lists to perform
> comparisons on the stripes they describe for each device. This is
> backwards and requires that we keep a transaction handle open while
> we're trimming.
> 
> This patchset adds an extent_io_tree to btrfs_device that maintains
> the allocation state of the device.  Extents are set dirty when
> chunks are first allocated -- when the extent maps are added to the
> mapping tree. They're cleared when last removed -- when the extent
> maps are removed from the mapping tree. This matches the lifespan
> of the pending and pinned chunks list and allows us to do trims
> on unallocated space safely without pinning the transaction for what
> may be a lengthy operation. We can also use this io tree to mark
> which chunks have already been trimmed so we don't repeat the operation.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/ctree.h            |  6 ---
>  fs/btrfs/disk-io.c          | 11 -----
>  fs/btrfs/extent-tree.c      | 28 -----------
>  fs/btrfs/extent_io.c        |  2 +-
>  fs/btrfs/extent_io.h        |  6 ++-
>  fs/btrfs/extent_map.c       | 36 ++++++++++++++
>  fs/btrfs/extent_map.h       |  1 -
>  fs/btrfs/free-space-cache.c |  4 --
>  fs/btrfs/transaction.c      |  9 ----
>  fs/btrfs/transaction.h      |  1 -
>  fs/btrfs/volumes.c          | 96 +++++++++++++------------------------
>  fs/btrfs/volumes.h          |  2 +
>  12 files changed, 76 insertions(+), 126 deletions(-)
> 
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -918,7 +918,7 @@ static void cache_state(struct extent_state *state,
>   * [start, end] is inclusive This takes the tree lock.
>   */
>  
> -static int __must_check
> +int __must_check
>  __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>  		 unsigned bits, unsigned exclusive_bits,
>  		 u64 *failed_start, struct extent_state **cached_state,

Does this really need to be exported again? There are helpers that can
wrap specific combinations of parameters.

> @@ -335,6 +335,8 @@ void btrfs_free_device(struct btrfs_device *device)
>  {
>  	WARN_ON(!list_empty(&device->post_commit_list));
>  	rcu_string_free(device->name);
> +	if (!in_softirq())
> +		extent_io_tree_release(&device->alloc_state);

This needs a comment

>  	bio_put(device->flush_bio);
>  	kfree(device);
>  }

The commit is quite big but I don't see how to shrink it, the changes
need to be done in several places. So, probably ok.