From: "Darrick J. Wong" <djwong@kernel.org>
To: aalbersh@kernel.org, djwong@kernel.org, cem@kernel.org
Cc: Gao Xiang <hsiangkao@linux.alibaba.com>,
Dave Chinner <dchinner@redhat.com>,
Chandan Babu R <chandanbabu@kernel.org>,
linux-xfs@vger.kernel.org
Subject: [PATCH 01/64] xfs: avoid redundant AGFL buffer invalidation
Date: Tue, 01 Oct 2024 18:08:05 -0700 [thread overview]
Message-ID: <172783101795.4036371.15102625450249242753.stgit@frogsfrogsfrogs> (raw)
In-Reply-To: <172783101710.4036371.10020616537589726441.stgit@frogsfrogsfrogs>
From: Darrick J. Wong <djwong@kernel.org>
Source kernel commit: d40c2865bdbbbba6418436b0a877daebe1d7c63e
Currently AGFL blocks can be filled from the following three sources:
- allocbt free blocks, as in xfs_allocbt_free_block();
- rmapbt free blocks, as in xfs_rmapbt_free_block();
- refilled from freespace btrees, as in xfs_alloc_fix_freelist().
Originally, allocbt free blocks would be marked as stale only when they
put back in the general free space pool as Dave mentioned on IRC, "we
don't stale AGF metadata btree blocks when they are returned to the
AGFL .. but once they get put back in the general free space pool, we
have to make sure the buffers are marked stale as the next user of
those blocks might be user data...."
However, after commit ca250b1b3d71 ("xfs: invalidate allocbt blocks
moved to the free list") and commit edfd9dd54921 ("xfs: move buffer
invalidation to xfs_btree_free_block"), even allocbt / bmapbt free
blocks will be invalidated immediately since they may fail to pass
V5 format validation on writeback even writeback to free space would be
safe.
IOWs, IMHO currently there is actually no difference of free blocks
between AGFL freespace pool and the general free space pool. So let's
avoid extra redundant AGFL buffer invalidation, since otherwise we're
currently facing unnecessary xfs_log_force() due to xfs_trans_binval()
again on buffers already marked as stale before as below:
[ 333.507469] Call Trace:
[ 333.507862] xfs_buf_find+0x371/0x6a0 <- xfs_buf_lock
[ 333.508451] xfs_buf_get_map+0x3f/0x230
[ 333.509062] xfs_trans_get_buf_map+0x11a/0x280
[ 333.509751] xfs_free_agfl_block+0xa1/0xd0
[ 333.510403] xfs_agfl_free_finish_item+0x16e/0x1d0
[ 333.511157] xfs_defer_finish_noroll+0x1ef/0x5c0
[ 333.511871] xfs_defer_finish+0xc/0xa0
[ 333.512471] xfs_itruncate_extents_flags+0x18a/0x5e0
[ 333.513253] xfs_inactive_truncate+0xb8/0x130
[ 333.513930] xfs_inactive+0x223/0x270
xfs_log_force() will take tens of milliseconds with AGF buffer locked.
It becomes an unnecessary long latency especially on our PMEM devices
with FSDAX enabled and fsops like xfs_reflink_find_shared() at the same
time are stuck due to the same AGF lock. Removing the double
invalidation on the AGFL blocks does not make this issue go away, but
this patch fixes for our workloads in reality and it should also work
by the code analysis.
Note that I'm not sure I need to remove another redundant one in
xfs_alloc_ag_vextent_small() since it's unrelated to our workloads.
Also fstests are passed with this patch.
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---
libxfs/defer_item.c | 4 ++--
libxfs/xfs_alloc.c | 28 +---------------------------
libxfs/xfs_alloc.h | 6 ++++--
3 files changed, 7 insertions(+), 31 deletions(-)
diff --git a/libxfs/defer_item.c b/libxfs/defer_item.c
index 8cdf57eac..77a368e6f 100644
--- a/libxfs/defer_item.c
+++ b/libxfs/defer_item.c
@@ -189,8 +189,8 @@ xfs_agfl_free_finish_item(
error = xfs_alloc_read_agf(xefi->xefi_pag, tp, 0, &agbp);
if (!error)
- error = xfs_free_agfl_block(tp, xefi->xefi_pag->pag_agno,
- agbno, agbp, &oinfo);
+ error = xfs_free_ag_extent(tp, agbp, xefi->xefi_pag->pag_agno,
+ agbno, 1, &oinfo, XFS_AG_RESV_AGFL);
xfs_extent_free_put_group(xefi);
kmem_cache_free(xfs_extfree_item_cache, xefi);
diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index 45feff034..ab547d80c 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -1928,7 +1928,7 @@ xfs_alloc_ag_vextent_size(
/*
* Free the extent starting at agno/bno for length.
*/
-STATIC int
+int
xfs_free_ag_extent(
struct xfs_trans *tp,
struct xfs_buf *agbp,
@@ -2418,32 +2418,6 @@ xfs_alloc_space_available(
return true;
}
-int
-xfs_free_agfl_block(
- struct xfs_trans *tp,
- xfs_agnumber_t agno,
- xfs_agblock_t agbno,
- struct xfs_buf *agbp,
- struct xfs_owner_info *oinfo)
-{
- int error;
- struct xfs_buf *bp;
-
- error = xfs_free_ag_extent(tp, agbp, agno, agbno, 1, oinfo,
- XFS_AG_RESV_AGFL);
- if (error)
- return error;
-
- error = xfs_trans_get_buf(tp, tp->t_mountp->m_ddev_targp,
- XFS_AGB_TO_DADDR(tp->t_mountp, agno, agbno),
- tp->t_mountp->m_bsize, 0, &bp);
- if (error)
- return error;
- xfs_trans_binval(tp, bp);
-
- return 0;
-}
-
/*
* Check the agfl fields of the agf for inconsistency or corruption.
*
diff --git a/libxfs/xfs_alloc.h b/libxfs/xfs_alloc.h
index 0b956f8b9..3dc8e44fe 100644
--- a/libxfs/xfs_alloc.h
+++ b/libxfs/xfs_alloc.h
@@ -80,6 +80,10 @@ int xfs_alloc_get_freelist(struct xfs_perag *pag, struct xfs_trans *tp,
int xfs_alloc_put_freelist(struct xfs_perag *pag, struct xfs_trans *tp,
struct xfs_buf *agfbp, struct xfs_buf *agflbp,
xfs_agblock_t bno, int btreeblk);
+int xfs_free_ag_extent(struct xfs_trans *tp, struct xfs_buf *agbp,
+ xfs_agnumber_t agno, xfs_agblock_t bno,
+ xfs_extlen_t len, const struct xfs_owner_info *oinfo,
+ enum xfs_ag_resv_type type);
/*
* Compute and fill in value of m_alloc_maxlevels.
@@ -194,8 +198,6 @@ int xfs_alloc_read_agf(struct xfs_perag *pag, struct xfs_trans *tp, int flags,
struct xfs_buf **agfbpp);
int xfs_alloc_read_agfl(struct xfs_perag *pag, struct xfs_trans *tp,
struct xfs_buf **bpp);
-int xfs_free_agfl_block(struct xfs_trans *, xfs_agnumber_t, xfs_agblock_t,
- struct xfs_buf *, struct xfs_owner_info *);
int xfs_alloc_fix_freelist(struct xfs_alloc_arg *args, uint32_t alloc_flags);
int xfs_free_extent_fix_freelist(struct xfs_trans *tp, struct xfs_perag *pag,
struct xfs_buf **agbp);
next prev parent reply other threads:[~2024-10-02 1:08 UTC|newest]
Thread overview: 111+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-02 1:00 [PATCHBOMB] xfsprogs: catch us up to 6.11 Darrick J. Wong
2024-10-02 1:04 ` [PATCHSET 1/6] xfsprogs: Debian and Ubuntu archive changes Darrick J. Wong
2024-10-02 1:05 ` [PATCH 1/6] debian: Update debhelper-compat level Darrick J. Wong
2024-10-02 1:06 ` [PATCH 2/6] debian: Update public release key Darrick J. Wong
2024-10-02 1:06 ` [PATCH 3/6] debian: Prevent recreating the orig tarball Darrick J. Wong
2024-10-02 1:06 ` [PATCH 4/6] debian: Add Build-Depends on pkg with systemd.pc Darrick J. Wong
2024-10-02 1:07 ` [PATCH 5/6] debian: Modernize build script Darrick J. Wong
2024-10-02 1:07 ` [PATCH 6/6] debian: Correct the day-of-week on 2024-09-04 Darrick J. Wong
2024-10-02 5:47 ` [PATCHSET 1/6] xfsprogs: Debian and Ubuntu archive changes Christoph Hellwig
2024-10-02 1:04 ` [PATCHSET 2/6] xfsprogs: do not depend on libattr Darrick J. Wong
2024-10-02 1:07 ` [PATCH 1/2] misc: clean up code around attr_list_by_handle calls Darrick J. Wong
2024-10-02 1:07 ` [PATCH 2/2] libfrog: emulate deprecated attrlist functionality in libattr Darrick J. Wong
2024-10-02 1:04 ` [PATCHSET v2.5 3/6] libxfs: resync with 6.11 Darrick J. Wong
2024-10-02 1:08 ` Darrick J. Wong [this message]
2024-10-02 1:08 ` [PATCH 02/64] xfs: don't walk off the end of a directory data block Darrick J. Wong
2024-10-02 1:08 ` [PATCH 03/64] xfs: Remove header files which are included more than once Darrick J. Wong
2024-10-02 1:08 ` [PATCH 04/64] xfs: hoist extent size helpers to libxfs Darrick J. Wong
2024-10-02 1:09 ` [PATCH 05/64] xfs: hoist inode flag conversion functions " Darrick J. Wong
2024-10-02 1:09 ` [PATCH 06/64] xfs: hoist project id get/set " Darrick J. Wong
2024-10-02 1:09 ` [PATCH 07/64] libxfs: put all the inode functions in a single file Darrick J. Wong
2024-10-02 1:09 ` [PATCH 08/64] libxfs: pass IGET flags through to xfs_iread Darrick J. Wong
2024-10-02 5:47 ` Christoph Hellwig
2024-10-02 1:10 ` [PATCH 09/64] xfs: pack icreate initialization parameters into a separate structure Darrick J. Wong
2024-10-02 1:10 ` [PATCH 10/64] libxfs: " Darrick J. Wong
2024-10-02 1:10 ` [PATCH 11/64] xfs: implement atime updates in xfs_trans_ichgtime Darrick J. Wong
2024-10-02 1:10 ` [PATCH 12/64] libxfs: rearrange libxfs_trans_ichgtime call when creating inodes Darrick J. Wong
2024-10-02 5:48 ` Christoph Hellwig
2024-10-02 1:11 ` [PATCH 13/64] libxfs: set access time when creating files Darrick J. Wong
2024-10-02 5:49 ` Christoph Hellwig
2024-10-02 1:11 ` [PATCH 14/64] libxfs: when creating a file in a directory, set the project id based on the parent Darrick J. Wong
2024-10-02 5:49 ` Christoph Hellwig
2024-10-02 1:11 ` [PATCH 15/64] libxfs: pass flags2 from parent to child when creating files Darrick J. Wong
2024-10-02 5:49 ` Christoph Hellwig
2024-10-02 1:12 ` [PATCH 16/64] xfs: split new inode creation into two pieces Darrick J. Wong
2024-10-02 1:12 ` [PATCH 17/64] libxfs: " Darrick J. Wong
2024-10-02 1:12 ` [PATCH 18/64] libxfs: backport inode init code from the kernel Darrick J. Wong
2024-10-02 5:50 ` Christoph Hellwig
2024-10-02 1:12 ` [PATCH 19/64] libxfs: remove libxfs_dir_ialloc Darrick J. Wong
2024-10-02 5:51 ` Christoph Hellwig
2024-10-02 1:13 ` [PATCH 20/64] libxfs: implement get_random_u32 Darrick J. Wong
2024-10-02 5:51 ` Christoph Hellwig
2024-10-02 1:13 ` [PATCH 21/64] xfs: hoist new inode initialization functions to libxfs Darrick J. Wong
2024-10-02 1:13 ` [PATCH 22/64] xfs: hoist xfs_iunlink " Darrick J. Wong
2024-10-02 1:13 ` [PATCH 23/64] xfs: hoist xfs_{bump,drop}link " Darrick J. Wong
2024-10-02 1:14 ` [PATCH 24/64] xfs: separate the icreate logic around INIT_XATTRS Darrick J. Wong
2024-10-02 1:14 ` [PATCH 25/64] xfs: create libxfs helper to link a new inode into a directory Darrick J. Wong
2024-10-02 1:14 ` [PATCH 26/64] xfs: create libxfs helper to link an existing " Darrick J. Wong
2024-10-02 1:14 ` [PATCH 27/64] xfs: hoist inode free function to libxfs Darrick J. Wong
2024-10-02 1:15 ` [PATCH 28/64] xfs: create libxfs helper to remove an existing inode/name from a directory Darrick J. Wong
2024-10-02 1:15 ` [PATCH 29/64] xfs: create libxfs helper to exchange two directory entries Darrick J. Wong
2024-10-02 1:15 ` [PATCH 30/64] xfs: create libxfs helper to rename " Darrick J. Wong
2024-10-02 1:15 ` [PATCH 31/64] xfs: move dirent update hooks to xfs_dir2.c Darrick J. Wong
2024-10-02 1:16 ` [PATCH 32/64] xfs: don't use the incore struct xfs_sb for offsets into struct xfs_dsb Darrick J. Wong
2024-10-02 1:16 ` [PATCH 33/64] xfs: clean up extent free log intent item tracepoint callsites Darrick J. Wong
2024-10-02 1:16 ` [PATCH 34/64] xfs: convert "skip_discard" to a proper flags bitset Darrick J. Wong
2024-10-02 1:16 ` [PATCH 35/64] xfs: pass the fsbno to xfs_perag_intent_get Darrick J. Wong
2024-10-02 1:17 ` [PATCH 36/64] xfs: add a xefi_entry helper Darrick J. Wong
2024-10-02 1:17 ` [PATCH 37/64] xfs: reuse xfs_extent_free_cancel_item Darrick J. Wong
2024-10-02 1:17 ` [PATCH 38/64] xfs: remove duplicate asserts in xfs_defer_extent_free Darrick J. Wong
2024-10-02 1:17 ` [PATCH 39/64] xfs: remove xfs_defer_agfl_block Darrick J. Wong
2024-10-02 1:18 ` [PATCH 40/64] xfs: move xfs_extent_free_defer_add to xfs_extfree_item.c Darrick J. Wong
2024-10-02 1:18 ` [PATCH 41/64] xfs: give rmap btree cursor error tracepoints their own class Darrick J. Wong
2024-10-02 1:18 ` [PATCH 42/64] xfs: pass btree cursors to rmap btree tracepoints Darrick J. Wong
2024-10-02 1:19 ` [PATCH 43/64] xfs: clean up rmap log intent item tracepoint callsites Darrick J. Wong
2024-10-02 1:19 ` [PATCH 44/64] xfs: add a ri_entry helper Darrick J. Wong
2024-10-02 1:19 ` [PATCH 45/64] xfs: reuse xfs_rmap_update_cancel_item Darrick J. Wong
2024-10-02 1:19 ` [PATCH 46/64] xfs: don't bother calling xfs_rmap_finish_one_cleanup in xfs_rmap_finish_one Darrick J. Wong
2024-10-02 1:20 ` [PATCH 47/64] xfs: simplify usage of the rcur local variable " Darrick J. Wong
2024-10-02 1:20 ` [PATCH 48/64] xfs: move xfs_rmap_update_defer_add to xfs_rmap_item.c Darrick J. Wong
2024-10-02 1:20 ` [PATCH 49/64] xfs: give refcount btree cursor error tracepoints their own class Darrick J. Wong
2024-10-02 1:20 ` [PATCH 50/64] xfs: create specialized classes for refcount tracepoints Darrick J. Wong
2024-10-02 1:21 ` [PATCH 51/64] xfs: pass btree cursors to refcount btree tracepoints Darrick J. Wong
2024-10-02 1:21 ` [PATCH 52/64] xfs: clean up refcount log intent item tracepoint callsites Darrick J. Wong
2024-10-02 1:21 ` [PATCH 53/64] xfs: add a ci_entry helper Darrick J. Wong
2024-10-02 1:21 ` [PATCH 54/64] xfs: reuse xfs_refcount_update_cancel_item Darrick J. Wong
2024-10-02 1:22 ` [PATCH 55/64] xfs: don't bother calling xfs_refcount_finish_one_cleanup in xfs_refcount_finish_one Darrick J. Wong
2024-10-02 1:22 ` [PATCH 56/64] xfs: simplify usage of the rcur local variable " Darrick J. Wong
2024-10-02 1:22 ` [PATCH 57/64] xfs: move xfs_refcount_update_defer_add to xfs_refcount_item.c Darrick J. Wong
2024-10-02 1:22 ` [PATCH 58/64] xfs: Avoid races with cnt_btree lastrec updates Darrick J. Wong
2024-10-02 1:23 ` [PATCH 59/64] xfs: AIL doesn't need manual pushing Darrick J. Wong
2024-10-02 1:23 ` [PATCH 60/64] xfs: background AIL push should target physical space Darrick J. Wong
2024-10-02 1:23 ` [PATCH 61/64] xfs: get rid of xfs_ag_resv_rmapbt_alloc Darrick J. Wong
2024-10-02 1:23 ` [PATCH 62/64] xfs: remove unused parameter in macro XFS_DQUOT_LOGRES Darrick J. Wong
2024-10-02 1:24 ` [PATCH 63/64] xfs: fix di_onlink checking for V1/V2 inodes Darrick J. Wong
2024-10-02 1:24 ` [PATCH 64/64] xfs: xfs_finobt_count_blocks() walks the wrong btree Darrick J. Wong
2024-10-02 1:05 ` [PATCHSET v2.5 4/6] xfsprogs: port tools to new 6.11 APIs Darrick J. Wong
2024-10-02 1:24 ` [PATCH 1/4] xfs_db: port the unlink command to use libxfs_droplink Darrick J. Wong
2024-10-02 5:52 ` Christoph Hellwig
2024-10-02 1:25 ` [PATCH 2/4] xfs_db/mkfs/xfs_repair: port to use XFS_ICREATE_UNLINKABLE Darrick J. Wong
2024-10-02 5:53 ` Christoph Hellwig
2024-10-02 22:50 ` Darrick J. Wong
2024-10-04 11:08 ` Andrey Albershteyn
2024-10-02 1:25 ` [PATCH 3/4] xfs_db/mdrestore/repair: don't use the incore struct xfs_sb for offsets into struct xfs_dsb Darrick J. Wong
2024-10-02 5:54 ` Christoph Hellwig
2024-10-02 22:51 ` Darrick J. Wong
2024-10-02 1:25 ` [PATCH 4/4] xfs_db: port the iunlink command to use the libxfs iunlink function Darrick J. Wong
2024-10-02 5:54 ` Christoph Hellwig
2024-10-02 1:05 ` [PATCHSET 5/6] xfs_repair: cleanups for 6.11 Darrick J. Wong
2024-10-02 1:25 ` [PATCH 1/4] xfs_repair: fix exchrange upgrade Darrick J. Wong
2024-10-02 5:57 ` Christoph Hellwig
2024-10-02 1:26 ` [PATCH 2/4] xfs_repair: don't crash in get_inode_parent Darrick J. Wong
2024-10-02 5:58 ` Christoph Hellwig
2024-10-02 1:26 ` [PATCH 3/4] xfs_repair: use library functions to reset root/rbm/rsum inodes Darrick J. Wong
2024-10-02 5:58 ` Christoph Hellwig
2024-10-02 1:26 ` [PATCH 4/4] xfs_repair: use library functions for orphanage creation Darrick J. Wong
2024-10-02 5:58 ` Christoph Hellwig
2024-10-02 1:05 ` [PATCHSET v2.5 6/6] mkfs: clean up inode initialization code Darrick J. Wong
2024-10-02 1:26 ` [PATCH 1/2] mkfs: clean up the rtinit() function Darrick J. Wong
2024-10-02 6:03 ` Christoph Hellwig
2024-10-02 1:27 ` [PATCH 2/2] mkfs: break up the rest of " Darrick J. Wong
2024-10-02 6:04 ` Christoph Hellwig
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=172783101795.4036371.15102625450249242753.stgit@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=aalbersh@kernel.org \
--cc=cem@kernel.org \
--cc=chandanbabu@kernel.org \
--cc=dchinner@redhat.com \
--cc=hsiangkao@linux.alibaba.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox