Re: [RFC PATCH v3 5/6] btrfs: qgroup: Introduce extent changeset for qgroup reserve functions

2017-05-11 Thread Qu Wenruo 



At 05/11/2017 01:59 AM, Goldwyn Rodrigues wrote:



On 05/09/2017 09:36 PM, Qu Wenruo wrote:

Introduce a new parameter, struct extent_changeset for
btrfs_qgroup_reserved_data() and its callers.

Such extent_changeset was used in btrfs_qgroup_reserve_data() to record
which range it reserved in current reserve, so it can free it at error
path.

The reason we need to export it to callers is, at buffered write error
path, without knowing what exactly which range we reserved in current
allocation, we can free space which is not reserved by us.

This will lead to qgroup reserved space underflow.

Reviewed-by: Chandan Rajendra 
Signed-off-by: Qu Wenruo 
---
  fs/btrfs/ctree.h   |  6 --
  fs/btrfs/extent-tree.c | 16 +++-
  fs/btrfs/extent_io.h   | 34 ++
  fs/btrfs/file.c| 12 +---
  fs/btrfs/inode-map.c   |  4 +++-
  fs/btrfs/inode.c   | 18 ++
  fs/btrfs/ioctl.c   |  5 -
  fs/btrfs/qgroup.c  | 41 +
  fs/btrfs/qgroup.h  |  3 ++-
  fs/btrfs/relocation.c  |  4 +++-
  10 files changed, 113 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1e82516fe2d8..52a0147cd612 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2704,8 +2704,9 @@ enum btrfs_flush_state {
COMMIT_TRANS=   6,
  };
  
-int btrfs_check_data_free_space(struct inode *inode, u64 start, u64 len);

  int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes);
+int btrfs_check_data_free_space(struct inode *inode,
+   struct extent_changeset **reserved, u64 start, u64 len);
  void btrfs_free_reserved_data_space(struct inode *inode, u64 start, u64 len);
  void btrfs_free_reserved_data_space_noquota(struct inode *inode, u64 start,
u64 len);
@@ -2723,7 +2724,8 @@ void btrfs_subvolume_release_metadata(struct 
btrfs_fs_info *fs_info,
  struct btrfs_block_rsv *rsv);
  int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes);
  void btrfs_delalloc_release_metadata(struct btrfs_inode *inode, u64 
num_bytes);
-int btrfs_delalloc_reserve_space(struct inode *inode, u64 start, u64 len);
+int btrfs_delalloc_reserve_space(struct inode *inode,
+   struct extent_changeset **reserved, u64 start, u64 len);
  void btrfs_delalloc_release_space(struct inode *inode, u64 start, u64 len);
  void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, unsigned short type);
  struct btrfs_block_rsv *btrfs_alloc_block_rsv(struct btrfs_fs_info *fs_info,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 4f62696131a6..782e0f5feb69 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3364,6 +3364,7 @@ static int cache_save_setup(struct 
btrfs_block_group_cache *block_group,
struct btrfs_fs_info *fs_info = block_group->fs_info;
struct btrfs_root *root = fs_info->tree_root;
struct inode *inode = NULL;
+   struct extent_changeset *data_reserved = NULL;
u64 alloc_hint = 0;
int dcs = BTRFS_DC_ERROR;
u64 num_pages = 0;
@@ -3483,7 +3484,7 @@ static int cache_save_setup(struct 
btrfs_block_group_cache *block_group,
num_pages *= 16;
num_pages *= PAGE_SIZE;
  
-	ret = btrfs_check_data_free_space(inode, 0, num_pages);

+   ret = btrfs_check_data_free_space(inode, _reserved, 0, num_pages);
if (ret)
goto out_put;
  
@@ -3514,6 +3515,7 @@ static int cache_save_setup(struct btrfs_block_group_cache *block_group,

block_group->disk_cache_state = dcs;
spin_unlock(_group->lock);
  
+	extent_changeset_free(data_reserved);

return ret;
  }
  
@@ -4282,7 +4284,8 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)

   * Will replace old btrfs_check_data_free_space(), but for patch split,
   * add a new function first and then replace it.
   */
-int btrfs_check_data_free_space(struct inode *inode, u64 start, u64 len)
+int btrfs_check_data_free_space(struct inode *inode,
+   struct extent_changeset **reserved, u64 start, u64 len)
  {
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
int ret;
@@ -4297,9 +4300,11 @@ int btrfs_check_data_free_space(struct inode *inode, u64 
start, u64 len)
return ret;
  
  	/* Use new btrfs_qgroup_reserve_data to reserve precious data space. */

-   ret = btrfs_qgroup_reserve_data(inode, start, len);
+   ret = btrfs_qgroup_reserve_data(inode, reserved, start, len);
if (ret < 0)
btrfs_free_reserved_data_space_noquota(inode, start, len);
+   else
+   ret = 0;
return ret;
  }
  
@@ -6140,11 +6145,12 @@ void btrfs_delalloc_release_metadata(struct btrfs_inode *inode, u64 num_bytes)

   * Return

Re: [RFC PATCH v3 5/6] btrfs: qgroup: Introduce extent changeset for qgroup reserve functions

2017-05-10 Thread Goldwyn Rodrigues 


On 05/09/2017 09:36 PM, Qu Wenruo wrote:
> Introduce a new parameter, struct extent_changeset for
> btrfs_qgroup_reserved_data() and its callers.
> 
> Such extent_changeset was used in btrfs_qgroup_reserve_data() to record
> which range it reserved in current reserve, so it can free it at error
> path.
> 
> The reason we need to export it to callers is, at buffered write error
> path, without knowing what exactly which range we reserved in current
> allocation, we can free space which is not reserved by us.
> 
> This will lead to qgroup reserved space underflow.
> 
> Reviewed-by: Chandan Rajendra 
> Signed-off-by: Qu Wenruo 
> ---
>  fs/btrfs/ctree.h   |  6 --
>  fs/btrfs/extent-tree.c | 16 +++-
>  fs/btrfs/extent_io.h   | 34 ++
>  fs/btrfs/file.c| 12 +---
>  fs/btrfs/inode-map.c   |  4 +++-
>  fs/btrfs/inode.c   | 18 ++
>  fs/btrfs/ioctl.c   |  5 -
>  fs/btrfs/qgroup.c  | 41 +
>  fs/btrfs/qgroup.h  |  3 ++-
>  fs/btrfs/relocation.c  |  4 +++-
>  10 files changed, 113 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 1e82516fe2d8..52a0147cd612 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2704,8 +2704,9 @@ enum btrfs_flush_state {
>   COMMIT_TRANS=   6,
>  };
>  
> -int btrfs_check_data_free_space(struct inode *inode, u64 start, u64 len);
>  int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes);
> +int btrfs_check_data_free_space(struct inode *inode,
> + struct extent_changeset **reserved, u64 start, u64 len);
>  void btrfs_free_reserved_data_space(struct inode *inode, u64 start, u64 len);
>  void btrfs_free_reserved_data_space_noquota(struct inode *inode, u64 start,
>   u64 len);
> @@ -2723,7 +2724,8 @@ void btrfs_subvolume_release_metadata(struct 
> btrfs_fs_info *fs_info,
> struct btrfs_block_rsv *rsv);
>  int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 
> num_bytes);
>  void btrfs_delalloc_release_metadata(struct btrfs_inode *inode, u64 
> num_bytes);
> -int btrfs_delalloc_reserve_space(struct inode *inode, u64 start, u64 len);
> +int btrfs_delalloc_reserve_space(struct inode *inode,
> + struct extent_changeset **reserved, u64 start, u64 len);
>  void btrfs_delalloc_release_space(struct inode *inode, u64 start, u64 len);
>  void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, unsigned short type);
>  struct btrfs_block_rsv *btrfs_alloc_block_rsv(struct btrfs_fs_info *fs_info,
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 4f62696131a6..782e0f5feb69 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3364,6 +3364,7 @@ static int cache_save_setup(struct 
> btrfs_block_group_cache *block_group,
>   struct btrfs_fs_info *fs_info = block_group->fs_info;
>   struct btrfs_root *root = fs_info->tree_root;
>   struct inode *inode = NULL;
> + struct extent_changeset *data_reserved = NULL;
>   u64 alloc_hint = 0;
>   int dcs = BTRFS_DC_ERROR;
>   u64 num_pages = 0;
> @@ -3483,7 +3484,7 @@ static int cache_save_setup(struct 
> btrfs_block_group_cache *block_group,
>   num_pages *= 16;
>   num_pages *= PAGE_SIZE;
>  
> - ret = btrfs_check_data_free_space(inode, 0, num_pages);
> + ret = btrfs_check_data_free_space(inode, _reserved, 0, num_pages);
>   if (ret)
>   goto out_put;
>  
> @@ -3514,6 +3515,7 @@ static int cache_save_setup(struct 
> btrfs_block_group_cache *block_group,
>   block_group->disk_cache_state = dcs;
>   spin_unlock(_group->lock);
>  
> + extent_changeset_free(data_reserved);
>   return ret;
>  }
>  
> @@ -4282,7 +4284,8 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode 
> *inode, u64 bytes)
>   * Will replace old btrfs_check_data_free_space(), but for patch split,
>   * add a new function first and then replace it.
>   */
> -int btrfs_check_data_free_space(struct inode *inode, u64 start, u64 len)
> +int btrfs_check_data_free_space(struct inode *inode,
> + struct extent_changeset **reserved, u64 start, u64 len)
>  {
>   struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>   int ret;
> @@ -4297,9 +4300,11 @@ int btrfs_check_data_free_space(struct inode *inode, 
> u64 start, u64 len)
>   return ret;
>  
>   /* Use new btrfs_qgroup_reserve_data to reserve precious data space. */
> - ret = btrfs_qgroup_reserve_data(inode, start, len);
> + ret = btrfs_qgroup_reserve_data(inode, reserved, start, len);
>   if (ret < 0)
>   btrfs_free_reserved_data_space_noquota(inode, start, len);
> + else
> + ret = 0;
>   return ret;
>  }
>  
> @@ -6140,11 +6145,12 @@ void