Skip to content

Commit 4ed6435

Browse files
author
Darrick J. Wong
committed
xfs: stop artificially limiting the length of bunmap calls
In commit e1a4e37, we clamped the length of bunmapi calls on the data forks of shared files to avoid two failure scenarios: one where the extent being unmapped is so sparsely shared that we exceed the transaction reservation with the sheer number of refcount btree updates and EFI intent items; and the other where we attach so many deferred updates to the transaction that we pin the log tail and later the log head meets the tail, causing the log to livelock. We avoid triggering the first problem by tracking the number of ops in the refcount btree cursor and forcing a requeue of the refcount intent item any time we think that we might be close to overflowing. This has been baked into XFS since before the original e1a4 patch. A recent patchset fixed the second problem by changing the deferred ops code to finish all the work items created by each round of trying to complete a refcount intent item, which eliminates the long chains of deferred items (27dad); and causing long-running transactions to relog their intent log items when space in the log gets low (74f4d). Because this clamp affects /any/ unmapping request regardless of the sharing factors of the component blocks, it degrades the performance of all large unmapping requests -- whereas with an unshared file we can unmap millions of blocks in one go, shared files are limited to unmapping a few thousand blocks at a time, which causes the upper level code to spin in a bunmapi loop even if it wasn't needed. This also eliminates one more place where log recovery behavior can differ from online behavior, because bunmapi operations no longer need to requeue. The fstest generic/447 was created to test the old fix, and it still passes with this applied. Partial-revert-of: e1a4e37 ("xfs: try to avoid blowing out the transaction reservation when bunmaping a shared extent") Depends: 27dada0 ("xfs: change the order in which child and parent defer ops ar finished") Depends: 74f4d6a ("xfs: only relog deferred intent items if free space in the log gets low") Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Dave Chinner <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]>
1 parent c47260d commit 4ed6435

File tree

2 files changed

+1
-26
lines changed

2 files changed

+1
-26
lines changed

fs/xfs/libxfs/xfs_bmap.c

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5280,7 +5280,6 @@ __xfs_bunmapi(
52805280
int whichfork; /* data or attribute fork */
52815281
xfs_fsblock_t sum;
52825282
xfs_filblks_t len = *rlen; /* length to unmap in file */
5283-
xfs_fileoff_t max_len;
52845283
xfs_fileoff_t end;
52855284
struct xfs_iext_cursor icur;
52865285
bool done = false;
@@ -5299,16 +5298,6 @@ __xfs_bunmapi(
52995298
ASSERT(len > 0);
53005299
ASSERT(nexts >= 0);
53015300

5302-
/*
5303-
* Guesstimate how many blocks we can unmap without running the risk of
5304-
* blowing out the transaction with a mix of EFIs and reflink
5305-
* adjustments.
5306-
*/
5307-
if (tp && xfs_is_reflink_inode(ip) && whichfork == XFS_DATA_FORK)
5308-
max_len = min(len, xfs_refcount_max_unmap(tp->t_log_res));
5309-
else
5310-
max_len = len;
5311-
53125301
error = xfs_iread_extents(tp, ip, whichfork);
53135302
if (error)
53145303
return error;
@@ -5347,7 +5336,7 @@ __xfs_bunmapi(
53475336

53485337
extno = 0;
53495338
while (end != (xfs_fileoff_t)-1 && end >= start &&
5350-
(nexts == 0 || extno < nexts) && max_len > 0) {
5339+
(nexts == 0 || extno < nexts)) {
53515340
/*
53525341
* Is the found extent after a hole in which end lives?
53535342
* Just back up to the previous extent, if so.
@@ -5381,14 +5370,6 @@ __xfs_bunmapi(
53815370
if (del.br_startoff + del.br_blockcount > end + 1)
53825371
del.br_blockcount = end + 1 - del.br_startoff;
53835372

5384-
/* How much can we safely unmap? */
5385-
if (max_len < del.br_blockcount) {
5386-
del.br_startoff += del.br_blockcount - max_len;
5387-
if (!wasdel)
5388-
del.br_startblock += del.br_blockcount - max_len;
5389-
del.br_blockcount = max_len;
5390-
}
5391-
53925373
if (!isrt)
53935374
goto delete;
53945375

@@ -5524,7 +5505,6 @@ __xfs_bunmapi(
55245505
if (error)
55255506
goto error0;
55265507

5527-
max_len -= del.br_blockcount;
55285508
end = del.br_startoff - 1;
55295509
nodelete:
55305510
/*

fs/xfs/libxfs/xfs_refcount.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,6 @@ extern int xfs_refcount_recover_cow_leftovers(struct xfs_mount *mp,
7878
*/
7979
#define XFS_REFCOUNT_ITEM_OVERHEAD 32
8080

81-
static inline xfs_fileoff_t xfs_refcount_max_unmap(int log_res)
82-
{
83-
return (log_res * 3 / 4) / XFS_REFCOUNT_ITEM_OVERHEAD;
84-
}
85-
8681
extern int xfs_refcount_has_record(struct xfs_btree_cur *cur,
8782
xfs_agblock_t bno, xfs_extlen_t len, bool *exists);
8883
union xfs_btree_rec;

0 commit comments

Comments
 (0)