All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: linux-xfs@vger.kernel.org
Cc: "Darrick J . Wong" <darrick.wong@oracle.com>
Subject: [RFC PATCH] xfs: transfer freed blocks to blk res when lazy accounting
Date: Fri, 22 May 2020 13:18:28 -0400	[thread overview]
Message-ID: <20200522171828.53440-1-bfoster@redhat.com> (raw)

Signed-off-by: Brian Foster <bfoster@redhat.com>
---

Darrick mentioned on IRC a few days ago that he'd seen an issue that
looked similar to the problem with the rmapbt based extent swap
algorithm when the associated inodes happen to bounce between extent and
btree format. That problem caused repeated bmapbt block allocations and
frees that exhausted the transaction block reservation across the
sequence of transaction rolls. The workaround for that was to use an
oversized block reservation, but that is not a generic or efficient
solution.

I was originally playing around with some hacks to set an optional base
block reservation on the transaction that we would attempt to replenish
across transaction roll sequences as the block reservation depletes, but
eventually noticed that there isn't much difference between stuffing
block frees in the transaction reservation counter vs. the delta counter
when lazy sb accounting is enabled (which is required for v5 supers). As
such, the following patch seems to address the rmapbt issue in my
isolated tests.

I think one tradeoff with this logic is that chains of rolling/freeing
transactions would now aggregate freed space until the final transaction
commits vs. as transactions roll. It's not immediately clear to me how
much of an issue that is, but it sounds a bit dicey when considering
things like truncates of large files. This behavior could still be tied
to a transaction flag to restrict its use to situations like rmapbt
swapext, however. Anyways, this is mostly untested outside of the extent
swap use case so I wanted to throw this on the list as an RFC for now
and see if anybody has thoughts or other ideas.

Brian

 fs/xfs/xfs_bmap_util.c | 11 -----------
 fs/xfs/xfs_trans.c     |  4 ++++
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index f37f5cc4b19f..74b3bad6c414 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1628,17 +1628,6 @@ xfs_swap_extents(
 		 */
 		resblks = XFS_SWAP_RMAP_SPACE_RES(mp, ipnext, w);
 		resblks +=  XFS_SWAP_RMAP_SPACE_RES(mp, tipnext, w);
-
-		/*
-		 * Handle the corner case where either inode might straddle the
-		 * btree format boundary. If so, the inode could bounce between
-		 * btree <-> extent format on unmap -> remap cycles, freeing and
-		 * allocating a bmapbt block each time.
-		 */
-		if (ipnext == (XFS_IFORK_MAXEXT(ip, w) + 1))
-			resblks += XFS_IFORK_MAXEXT(ip, w);
-		if (tipnext == (XFS_IFORK_MAXEXT(tip, w) + 1))
-			resblks += XFS_IFORK_MAXEXT(tip, w);
 	}
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0, 0, &tp);
 	if (error)
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 28b983ff8b11..b421d27445c1 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -370,6 +370,10 @@ xfs_trans_mod_sb(
 			tp->t_blk_res_used += (uint)-delta;
 			if (tp->t_blk_res_used > tp->t_blk_res)
 				xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+		} else if (delta > 0 &&
+			   xfs_sb_version_haslazysbcount(&mp->m_sb)) {
+			tp->t_blk_res += delta;
+			delta = 0;
 		}
 		tp->t_fdblocks_delta += delta;
 		if (xfs_sb_version_haslazysbcount(&mp->m_sb))
-- 
2.21.1


             reply	other threads:[~2020-05-22 17:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-22 17:18 Brian Foster [this message]
2020-05-23  1:36 ` [RFC PATCH] xfs: transfer freed blocks to blk res when lazy accounting Darrick J. Wong
2020-05-26 18:16   ` Brian Foster
2020-05-26 21:11     ` Darrick J. Wong
2020-05-27 12:27       ` Brian Foster
2020-05-27 15:31         ` Darrick J. Wong
2020-05-28 13:05           ` Brian Foster
2020-05-28 17:29             ` Darrick J. Wong
2020-05-29 11:33               ` Brian Foster
2020-05-30  1:16                 ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200522171828.53440-1-bfoster@redhat.com \
    --to=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.