* [PATCH 1/2] xfs: update icsb freeblocks counter after extent deletion
2014-10-14 21:59 [PATCH 0/2] fix up indlen reservations on extent split Brian Foster
@ 2014-10-14 21:59 ` Brian Foster
2014-10-14 21:59 ` [PATCH 2/2] xfs: borrow indirect blocks from freed extent when available Brian Foster
2014-10-29 16:38 ` [PATCH 0/2] fix up indlen reservations on extent split Brian Foster
2 siblings, 0 replies; 5+ messages in thread
From: Brian Foster @ 2014-10-14 21:59 UTC (permalink / raw)
To: xfs
xfs_bunmapi() currently updates the icsb fdblocks counter, unreserves
quota, etc. before the extent is deleted by xfs_bmap_del_extent(). The
function has problems dividing up the indirect reserved blocks for
scenarios where a single delalloc extent is split in two. Particularly,
there aren't always enough blocks reserved for multiple extents in a
single extent reservation.
The solution to this problem is to allow the extent removal code to
steal from the deleted extent to meet indirect reservation requirements.
Move the block of code in xfs_bmapi() that updates the icsb fdblocks
counter to after the call to xfs_bmap_del_extent() to allow the codepath
to update the extent record before the free blocks are accounted. Also,
reshuffle the code slightly so the delalloc accounting occurs near the
xfs_bmap_del_extent() call to provide context for the comments.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 59 ++++++++++++++++++++++++++++--------------------
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 79c9819..5abd22b 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5277,9 +5277,37 @@ xfs_bunmapi(
goto nodelete;
}
}
+
+ /*
+ * If it's the case where the directory code is running
+ * with no block reservation, and the deleted block is in
+ * the middle of its extent, and the resulting insert
+ * of an extent would cause transformation to btree format,
+ * then reject it. The calling code will then swap
+ * blocks around instead.
+ * We have to do this now, rather than waiting for the
+ * conversion to btree format, since the transaction
+ * will be dirty.
+ */
+ if (!wasdel && xfs_trans_get_block_res(tp) == 0 &&
+ XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_EXTENTS &&
+ XFS_IFORK_NEXTENTS(ip, whichfork) >= /* Note the >= */
+ XFS_IFORK_MAXEXT(ip, whichfork) &&
+ del.br_startoff > got.br_startoff &&
+ del.br_startoff + del.br_blockcount <
+ got.br_startoff + got.br_blockcount) {
+ error = -ENOSPC;
+ goto error0;
+ }
+
+ /*
+ * Unreserve quota and update realtime free space, if
+ * appropriate. If delayed allocation, update the inode delalloc
+ * counter now and wait to update the sb counters as
+ * xfs_bmap_del_extent() might need to borrow some blocks.
+ */
if (wasdel) {
ASSERT(startblockval(del.br_startblock) > 0);
- /* Update realtime/data freespace, unreserve quota */
if (isrt) {
xfs_filblks_t rtexts;
@@ -5291,8 +5319,6 @@ xfs_bunmapi(
ip, -((long)del.br_blockcount), 0,
XFS_QMOPT_RES_RTBLKS);
} else {
- xfs_icsb_modify_counters(mp, XFS_SBS_FDBLOCKS,
- (int64_t)del.br_blockcount, 0);
(void)xfs_trans_reserve_quota_nblks(NULL,
ip, -((long)del.br_blockcount), 0,
XFS_QMOPT_RES_REGBLKS);
@@ -5303,32 +5329,17 @@ xfs_bunmapi(
XFS_BTCUR_BPRV_WASDEL;
} else if (cur)
cur->bc_private.b.flags &= ~XFS_BTCUR_BPRV_WASDEL;
- /*
- * If it's the case where the directory code is running
- * with no block reservation, and the deleted block is in
- * the middle of its extent, and the resulting insert
- * of an extent would cause transformation to btree format,
- * then reject it. The calling code will then swap
- * blocks around instead.
- * We have to do this now, rather than waiting for the
- * conversion to btree format, since the transaction
- * will be dirty.
- */
- if (!wasdel && xfs_trans_get_block_res(tp) == 0 &&
- XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_NEXTENTS(ip, whichfork) >= /* Note the >= */
- XFS_IFORK_MAXEXT(ip, whichfork) &&
- del.br_startoff > got.br_startoff &&
- del.br_startoff + del.br_blockcount <
- got.br_startoff + got.br_blockcount) {
- error = -ENOSPC;
- goto error0;
- }
+
error = xfs_bmap_del_extent(ip, tp, &lastx, flist, cur, &del,
&tmp_logflags, whichfork);
logflags |= tmp_logflags;
if (error)
goto error0;
+
+ if (!isrt && wasdel)
+ xfs_icsb_modify_counters(mp, XFS_SBS_FDBLOCKS,
+ (int64_t) del.br_blockcount, 0);
+
bno = del.br_startoff - 1;
nodelete:
/*
--
1.8.3.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] xfs: borrow indirect blocks from freed extent when available
2014-10-14 21:59 [PATCH 0/2] fix up indlen reservations on extent split Brian Foster
2014-10-14 21:59 ` [PATCH 1/2] xfs: update icsb freeblocks counter after extent deletion Brian Foster
@ 2014-10-14 21:59 ` Brian Foster
2014-10-29 16:38 ` [PATCH 0/2] fix up indlen reservations on extent split Brian Foster
2 siblings, 0 replies; 5+ messages in thread
From: Brian Foster @ 2014-10-14 21:59 UTC (permalink / raw)
To: xfs
xfs_bmap_del_extent() handles extent removal from the in-core and
on-disk extent lists. When removing a delalloc range, it updates the
indirect block reservation appropriately based on the removal. It
currently enforces that the new indirect block reservation is less than
or equal to the original. This is normally the case in all situations
except for in certain cases when the removed range creates a hole in a
single delalloc extent, thus splitting a single delalloc extent in two.
It is possible with small enough extents to split an indlen==1 extent
into two such slightly smaller extents. This leaves one extent with 0
indirect blocks and leads to assert failures in other areas (e.g.,
xfs_bunmapi() if the extent happens to be removed).
Refactor the indirect reservation code into a new helper function
invoked by xfs_bunmapi(). Allow the helper to steal blocks from the
deleted extent if necessary to satisfy the worst case indirect
reservation of the new extents. This is safe now that the caller does
not update icsb counters until the extent is removed (such stolen blocks
simply remain accounted as allocated). Fall back to the original
reservation using the existing algorithm and warn if we end up with
extents without any reservation at all to detect this more easily in the
future.
This allows generic/033 to run on XFS without assert failures.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 100 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 81 insertions(+), 19 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 5abd22b..64b88b6 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4701,6 +4701,68 @@ error0:
}
/*
+ * When a delalloc extent is split (e.g., due to a hole punch), the original
+ * indlen reservation must be shared across the two new extents that are left
+ * behind.
+ *
+ * Provided the worst case indlen for two extents, limit the total reservation
+ * against that of the original extent. We may want to steal blocks from any
+ * extra that might be available according to the caller to make up a deficiency
+ * (e.g., if the original reservation consists of one block). The number of such
+ * stolen blocks is returned. The availability and accounting of stealable
+ * blocks is the responsibility of the caller.
+ */
+static xfs_filblks_t
+xfs_bmap_limit_indlen(
+ xfs_filblks_t ores, /* original res. */
+ xfs_filblks_t *indlen1, /* ext1 worst indlen */
+ xfs_filblks_t *indlen2, /* ext2 worst indlen */
+ xfs_filblks_t avail) /* stealable blocks */
+{
+ xfs_filblks_t nres; /* new total res. */
+ xfs_filblks_t temp;
+ xfs_filblks_t temp2;
+ xfs_filblks_t stolen = 0;
+
+ temp = *indlen1;
+ temp2 = *indlen2;
+ nres = temp + temp2;
+
+ /*
+ * Steal as many blocks as we can to try and satisfy the worst case
+ * indlen of both new extents.
+ */
+ while (nres > ores && avail) {
+ nres--;
+ avail--;
+ stolen++;
+ }
+
+ /*
+ * If we've exhausted stealable blocks and still haven't satisfied the
+ * worst case reservation, we have no choice but to pare back until
+ * covered by the original reservation.
+ */
+ while (nres > ores) {
+ if (temp) {
+ temp--;
+ nres--;
+ }
+ if (nres == ores)
+ break;
+ if (temp2) {
+ temp2--;
+ nres--;
+ }
+ }
+
+ *indlen1 = temp;
+ *indlen2 = temp2;
+
+ return stolen;
+}
+
+/*
* Called by xfs_bmapi to update file extent records and the btree
* after removing space (or undoing a delayed allocation).
*/
@@ -4963,28 +5025,28 @@ xfs_bmap_del_extent(
XFS_IFORK_NEXT_SET(ip, whichfork,
XFS_IFORK_NEXTENTS(ip, whichfork) + 1);
} else {
+ xfs_filblks_t stolen;
ASSERT(whichfork == XFS_DATA_FORK);
- temp = xfs_bmap_worst_indlen(ip, temp);
+
+ /*
+ * Fix up the indlen reservations. We can safely steal
+ * blocks from the deleted extent as this simply fudges
+ * the icsb fdblocks accounting in xfs_bunmapi().
+ */
+ temp = xfs_bmap_worst_indlen(ip, got.br_blockcount);
+ temp2 = xfs_bmap_worst_indlen(ip, new.br_blockcount);
+ stolen = xfs_bmap_limit_indlen(da_old, &temp, &temp2,
+ del->br_blockcount);
+ da_new = temp + temp2 - stolen;
+ del->br_blockcount -= stolen;
+
+ /*
+ * Set the reservation for each extent. Warn if either
+ * is zero as this can lead to delalloc problems.
+ */
+ WARN_ON_ONCE(!temp || !temp2);
xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
- temp2 = xfs_bmap_worst_indlen(ip, temp2);
new.br_startblock = nullstartblock((int)temp2);
- da_new = temp + temp2;
- while (da_new > da_old) {
- if (temp) {
- temp--;
- da_new--;
- xfs_bmbt_set_startblock(ep,
- nullstartblock((int)temp));
- }
- if (da_new == da_old)
- break;
- if (temp2) {
- temp2--;
- da_new--;
- new.br_startblock =
- nullstartblock((int)temp2);
- }
- }
}
trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
xfs_iext_insert(ip, *idx + 1, 1, &new, state);
--
1.8.3.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 5+ messages in thread