Linux-BTRFS Archive on lore.kernel.org
 help / Atom feed
* [PATCH] btrfs: qgroup: Make qgroup async transaction commit more aggressive
@ 2019-01-24 23:55 Qu Wenruo
  2019-02-12 11:09 ` Qu Wenruo
  0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2019-01-24 23:55 UTC (permalink / raw)
  To: linux-btrfs

[BUG]
Btrfs qgroup will still hit EDQUOT under the following case:

  #!/bin/bash

  dev=/dev/test/test
  mnt=/mnt/btrfs
  umount $mnt &> /dev/null
  umount $dev &> /dev/null

  mkfs.btrfs -f $dev
  mount $dev $mnt -o nospace_cache

  btrfs subv create $mnt/subv
  btrfs quota enable $mnt
  btrfs quota rescan -w $mnt
  btrfs qgroup limit -e 1G $mnt/subv

  fallocate -l 900M $mnt/subv/padding
  sync

  rm $mnt/subv/padding

  # Hit EDQUOT
  xfs_io -f -c "pwrite 0 512M" $mnt/subv/real_file

[CAUSE]
Since commit a514d63882c3 ("btrfs: qgroup: Commit transaction in advance
to reduce early EDQUOT"), btrfs is not forced to commit transaction to
reclaim more quota space.

Instead, we just check pertrans metadata reservation against some
threshold and try to do asynchronously transaction commit.

However in above case, the pertrans metadata reservation is pretty small
thus it will never trigger asynchronous transaction commit.

[FIX]
Instead of only accounting pertrans metadata reservation, we calculate
how much free space we have, and if there isn't much free space left,
commit transaction asynchronously to try to free some space.

This may slow down the fs when we have less than 32M free qgroup space,
but should reduce a lot of false EDQUOT, so the cost should be
acceptable.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 4e473a998219..f0b7425bf289 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2843,15 +2843,15 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
  * Two limits to commit transaction in advance.
  *
  * For RATIO, it will be 1/RATIO of the remaining limit
- * (excluding data and prealloc meta) as threshold.
+ * as threshold.
  * For SIZE, it will be in byte unit as threshold.
  */
-#define QGROUP_PERTRANS_RATIO		32
-#define QGROUP_PERTRANS_SIZE		SZ_32M
+#define QGROUP_FREE_RATIO		32
+#define QGROUP_FREE_SIZE		SZ_32M
 static bool qgroup_check_limits(struct btrfs_fs_info *fs_info,
 				const struct btrfs_qgroup *qg, u64 num_bytes)
 {
-	u64 limit;
+	u64 free;
 	u64 threshold;
 
 	if ((qg->lim_flags & BTRFS_QGROUP_LIMIT_MAX_RFER) &&
@@ -2870,20 +2870,23 @@ static bool qgroup_check_limits(struct btrfs_fs_info *fs_info,
 	 */
 	if ((qg->lim_flags & (BTRFS_QGROUP_LIMIT_MAX_RFER |
 			      BTRFS_QGROUP_LIMIT_MAX_EXCL))) {
-		if (qg->lim_flags & BTRFS_QGROUP_LIMIT_MAX_EXCL)
-			limit = qg->max_excl;
-		else
-			limit = qg->max_rfer;
-		threshold = (limit - qg->rsv.values[BTRFS_QGROUP_RSV_DATA] -
-			    qg->rsv.values[BTRFS_QGROUP_RSV_META_PREALLOC]) /
-			    QGROUP_PERTRANS_RATIO;
-		threshold = min_t(u64, threshold, QGROUP_PERTRANS_SIZE);
+		if (qg->lim_flags & BTRFS_QGROUP_LIMIT_MAX_EXCL) {
+			free = (qg->max_excl - qgroup_rsv_total(qg) -
+				     qg->excl);
+			threshold = min_t(u64, qg->max_excl / QGROUP_FREE_RATIO,
+					  QGROUP_FREE_SIZE);
+		} else {
+			free = (qg->max_rfer - qgroup_rsv_total(qg) -
+				     qg->rfer);
+			threshold = min_t(u64, qg->max_rfer / QGROUP_FREE_RATIO,
+					  QGROUP_FREE_SIZE);
+		}
 
 		/*
 		 * Use transaction_kthread to commit transaction, so we no
 		 * longer need to bother nested transaction nor lock context.
 		 */
-		if (qg->rsv.values[BTRFS_QGROUP_RSV_META_PERTRANS] > threshold)
+		if (free < threshold)
 			btrfs_commit_transaction_locksafe(fs_info);
 	}
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] btrfs: qgroup: Make qgroup async transaction commit more aggressive
  2019-01-24 23:55 [PATCH] btrfs: qgroup: Make qgroup async transaction commit more aggressive Qu Wenruo
@ 2019-02-12 11:09 ` Qu Wenruo
  2019-02-12 14:09   ` David Sterba
  0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2019-02-12 11:09 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs

[-- Attachment #1.1: Type: text/plain, Size: 3935 bytes --]

Gentle ping?

It would be pretty nice to make it into the next merge windows (v5.1).

Thanks,
Qu

On 2019/1/25 上午7:55, Qu Wenruo wrote:
> [BUG]
> Btrfs qgroup will still hit EDQUOT under the following case:
> 
>   #!/bin/bash
> 
>   dev=/dev/test/test
>   mnt=/mnt/btrfs
>   umount $mnt &> /dev/null
>   umount $dev &> /dev/null
> 
>   mkfs.btrfs -f $dev
>   mount $dev $mnt -o nospace_cache
> 
>   btrfs subv create $mnt/subv
>   btrfs quota enable $mnt
>   btrfs quota rescan -w $mnt
>   btrfs qgroup limit -e 1G $mnt/subv
> 
>   fallocate -l 900M $mnt/subv/padding
>   sync
> 
>   rm $mnt/subv/padding
> 
>   # Hit EDQUOT
>   xfs_io -f -c "pwrite 0 512M" $mnt/subv/real_file
> 
> [CAUSE]
> Since commit a514d63882c3 ("btrfs: qgroup: Commit transaction in advance
> to reduce early EDQUOT"), btrfs is not forced to commit transaction to
> reclaim more quota space.
> 
> Instead, we just check pertrans metadata reservation against some
> threshold and try to do asynchronously transaction commit.
> 
> However in above case, the pertrans metadata reservation is pretty small
> thus it will never trigger asynchronous transaction commit.
> 
> [FIX]
> Instead of only accounting pertrans metadata reservation, we calculate
> how much free space we have, and if there isn't much free space left,
> commit transaction asynchronously to try to free some space.
> 
> This may slow down the fs when we have less than 32M free qgroup space,
> but should reduce a lot of false EDQUOT, so the cost should be
> acceptable.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/qgroup.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 4e473a998219..f0b7425bf289 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2843,15 +2843,15 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
>   * Two limits to commit transaction in advance.
>   *
>   * For RATIO, it will be 1/RATIO of the remaining limit
> - * (excluding data and prealloc meta) as threshold.
> + * as threshold.
>   * For SIZE, it will be in byte unit as threshold.
>   */
> -#define QGROUP_PERTRANS_RATIO		32
> -#define QGROUP_PERTRANS_SIZE		SZ_32M
> +#define QGROUP_FREE_RATIO		32
> +#define QGROUP_FREE_SIZE		SZ_32M
>  static bool qgroup_check_limits(struct btrfs_fs_info *fs_info,
>  				const struct btrfs_qgroup *qg, u64 num_bytes)
>  {
> -	u64 limit;
> +	u64 free;
>  	u64 threshold;
>  
>  	if ((qg->lim_flags & BTRFS_QGROUP_LIMIT_MAX_RFER) &&
> @@ -2870,20 +2870,23 @@ static bool qgroup_check_limits(struct btrfs_fs_info *fs_info,
>  	 */
>  	if ((qg->lim_flags & (BTRFS_QGROUP_LIMIT_MAX_RFER |
>  			      BTRFS_QGROUP_LIMIT_MAX_EXCL))) {
> -		if (qg->lim_flags & BTRFS_QGROUP_LIMIT_MAX_EXCL)
> -			limit = qg->max_excl;
> -		else
> -			limit = qg->max_rfer;
> -		threshold = (limit - qg->rsv.values[BTRFS_QGROUP_RSV_DATA] -
> -			    qg->rsv.values[BTRFS_QGROUP_RSV_META_PREALLOC]) /
> -			    QGROUP_PERTRANS_RATIO;
> -		threshold = min_t(u64, threshold, QGROUP_PERTRANS_SIZE);
> +		if (qg->lim_flags & BTRFS_QGROUP_LIMIT_MAX_EXCL) {
> +			free = (qg->max_excl - qgroup_rsv_total(qg) -
> +				     qg->excl);
> +			threshold = min_t(u64, qg->max_excl / QGROUP_FREE_RATIO,
> +					  QGROUP_FREE_SIZE);
> +		} else {
> +			free = (qg->max_rfer - qgroup_rsv_total(qg) -
> +				     qg->rfer);
> +			threshold = min_t(u64, qg->max_rfer / QGROUP_FREE_RATIO,
> +					  QGROUP_FREE_SIZE);
> +		}
>  
>  		/*
>  		 * Use transaction_kthread to commit transaction, so we no
>  		 * longer need to bother nested transaction nor lock context.
>  		 */
> -		if (qg->rsv.values[BTRFS_QGROUP_RSV_META_PERTRANS] > threshold)
> +		if (free < threshold)
>  			btrfs_commit_transaction_locksafe(fs_info);
>  	}
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] btrfs: qgroup: Make qgroup async transaction commit more aggressive
  2019-02-12 11:09 ` Qu Wenruo
@ 2019-02-12 14:09   ` David Sterba
  0 siblings, 0 replies; 3+ messages in thread
From: David Sterba @ 2019-02-12 14:09 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs

On Tue, Feb 12, 2019 at 07:09:54PM +0800, Qu Wenruo wrote:
> Gentle ping?
> 
> It would be pretty nice to make it into the next merge windows (v5.1).

5.1 is ok, I'll add the patch to for-next. Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-24 23:55 [PATCH] btrfs: qgroup: Make qgroup async transaction commit more aggressive Qu Wenruo
2019-02-12 11:09 ` Qu Wenruo
2019-02-12 14:09   ` David Sterba

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox