Commit e1a4e37c authored by Darrick J. Wong's avatar Darrick J. Wong
Browse files

xfs: try to avoid blowing out the transaction reservation when bunmaping a shared extent 



In a pathological scenario where we are trying to bunmapi a single
extent in which every other block is shared, it's possible that trying
to unmap the entire large extent in a single transaction can generate so
many EFIs that we overflow the transaction reservation.

Therefore, use a heuristic to guess at the number of blocks we can
safely unmap from a reflink file's data fork in an single transaction.
This should prevent problems such as the log head slamming into the tail
and ASSERTs that trigger because we've exceeded the transaction
reservation.

Note that since bunmapi can fail to unmap the entire range, we must also
teach the deferred unmap code to roll into a new transaction whenever we
get low on reservation.

Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
[hch: random edits, all bugs are my fault]
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
parent d205a7d0

fs/xfs/libxfs/xfs_bmap.c

+30 −8
Original line number Diff line number Diff line
@@ -5434,6 +5434,7 @@ __xfs_bunmapi( 
	int			whichfork;	/* data or attribute fork */

	xfs_fsblock_t		sum;

	xfs_filblks_t		len = *rlen;	/* length to unmap in file */

	xfs_fileoff_t		max_len;



	trace_xfs_bunmap(ip, bno, len, flags, _RET_IP_);
@@ -5455,6 +5456,16 @@ __xfs_bunmapi( 
	ASSERT(len > 0);

	ASSERT(nexts >= 0);



	/*

	 * Guesstimate how many blocks we can unmap without running the risk of

	 * blowing out the transaction with a mix of EFIs and reflink

	 * adjustments.

	 */

	if (xfs_is_reflink_inode(ip) && whichfork == XFS_DATA_FORK)

		max_len = min(len, xfs_refcount_max_unmap(tp->t_log_res));

	else

		max_len = len;



	if (!(ifp->if_flags & XFS_IFEXTENTS) &&

	    (error = xfs_iread_extents(tp, ip, whichfork)))

		return error;
@@ -5499,7 +5510,7 @@ __xfs_bunmapi( 


	extno = 0;

	while (bno != (xfs_fileoff_t)-1 && bno >= start && lastx >= 0 &&

	       (nexts == 0 || extno < nexts)) {

	       (nexts == 0 || extno < nexts) && max_len > 0) {

		/*

		 * Is the found extent after a hole in which bno lives?

		 * Just back up to the previous extent, if so.
@@ -5531,6 +5542,15 @@ __xfs_bunmapi( 
		}

		if (del.br_startoff + del.br_blockcount > bno + 1)

			del.br_blockcount = bno + 1 - del.br_startoff;



		/* How much can we safely unmap? */

		if (max_len < del.br_blockcount) {

			del.br_startoff += del.br_blockcount - max_len;

			if (!wasdel)

				del.br_startblock += del.br_blockcount - max_len;

			del.br_blockcount = max_len;

		}



		sum = del.br_startblock + del.br_blockcount;

		if (isrt &&

		    (mod = do_mod(sum, mp->m_sb.sb_rextsize))) {
@@ -5707,6 +5727,7 @@ __xfs_bunmapi( 
		if (!isrt && wasdel)

			xfs_mod_fdblocks(mp, (int64_t)del.br_blockcount, false);



		max_len -= del.br_blockcount;

		bno = del.br_startoff - 1;

nodelete:

		/*
@@ -6472,15 +6493,16 @@ xfs_bmap_finish_one( 
	int				whichfork,

	xfs_fileoff_t			startoff,

	xfs_fsblock_t			startblock,

	xfs_filblks_t			blockcount,

	xfs_filblks_t			*blockcount,

	xfs_exntst_t			state)

{

	int				error = 0, done;

	xfs_fsblock_t			firstfsb;

	int				error = 0;



	trace_xfs_bmap_deferred(tp->t_mountp,

			XFS_FSB_TO_AGNO(tp->t_mountp, startblock), type,

			XFS_FSB_TO_AGBNO(tp->t_mountp, startblock),

			ip->i_ino, whichfork, startoff, blockcount, state);

			ip->i_ino, whichfork, startoff, *blockcount, state);



	if (WARN_ON_ONCE(whichfork != XFS_DATA_FORK))

		return -EFSCORRUPTED;
@@ -6492,13 +6514,13 @@ xfs_bmap_finish_one( 


	switch (type) {

	case XFS_BMAP_MAP:

		error = xfs_bmapi_remap(tp, ip, startoff, blockcount,

		error = xfs_bmapi_remap(tp, ip, startoff, *blockcount,

				startblock, dfops);

		*blockcount = 0;

		break;

	case XFS_BMAP_UNMAP:

		error = xfs_bunmapi(tp, ip, startoff, blockcount,

				XFS_BMAPI_REMAP, 1, &startblock, dfops, &done);

		ASSERT(done);

		error = __xfs_bunmapi(tp, ip, startoff, blockcount,

				XFS_BMAPI_REMAP, 1, &firstfsb, dfops);

		break;

	default:

		ASSERT(0);

fs/xfs/libxfs/xfs_bmap.h

+1 −1
Original line number Diff line number Diff line
@@ -271,7 +271,7 @@ struct xfs_bmap_intent { 
int	xfs_bmap_finish_one(struct xfs_trans *tp, struct xfs_defer_ops *dfops,

		struct xfs_inode *ip, enum xfs_bmap_intent_type type,

		int whichfork, xfs_fileoff_t startoff, xfs_fsblock_t startblock,

		xfs_filblks_t blockcount, xfs_exntst_t state);

		xfs_filblks_t *blockcount, xfs_exntst_t state);

int	xfs_bmap_map_extent(struct xfs_mount *mp, struct xfs_defer_ops *dfops,

		struct xfs_inode *ip, struct xfs_bmbt_irec *imap);

int	xfs_bmap_unmap_extent(struct xfs_mount *mp, struct xfs_defer_ops *dfops,

fs/xfs/libxfs/xfs_refcount.c

+1 −9
Original line number Diff line number Diff line
@@ -784,14 +784,6 @@ xfs_refcount_merge_extents( 
}



/*

 * While we're adjusting the refcounts records of an extent, we have

 * to keep an eye on the number of extents we're dirtying -- run too

 * many in a single transaction and we'll exceed the transaction's

 * reservation and crash the fs.  Each record adds 12 bytes to the

 * log (plus any key updates) so we'll conservatively assume 24 bytes

 * per record.  We must also leave space for btree splits on both ends

 * of the range and space for the CUD and a new CUI.

 *

 * XXX: This is a pretty hand-wavy estimate.  The penalty for guessing

 * true incorrectly is a shutdown FS; the penalty for guessing false

 * incorrectly is more transaction rolls than might be necessary.
@@ -822,7 +814,7 @@ xfs_refcount_still_have_space( 
	else if (overhead > cur->bc_tp->t_log_res)

		return false;

	return  cur->bc_tp->t_log_res - overhead >

		cur->bc_private.a.priv.refc.nr_ops * 32;

		cur->bc_private.a.priv.refc.nr_ops * XFS_REFCOUNT_ITEM_OVERHEAD;

}



/*

fs/xfs/libxfs/xfs_refcount.h

+16 −0
Original line number Diff line number Diff line
@@ -67,4 +67,20 @@ extern int xfs_refcount_free_cow_extent(struct xfs_mount *mp, 
extern int xfs_refcount_recover_cow_leftovers(struct xfs_mount *mp,

		xfs_agnumber_t agno);



/*

 * While we're adjusting the refcounts records of an extent, we have

 * to keep an eye on the number of extents we're dirtying -- run too

 * many in a single transaction and we'll exceed the transaction's

 * reservation and crash the fs.  Each record adds 12 bytes to the

 * log (plus any key updates) so we'll conservatively assume 32 bytes

 * per record.  We must also leave space for btree splits on both ends

 * of the range and space for the CUD and a new CUI.

 */

#define XFS_REFCOUNT_ITEM_OVERHEAD	32



static inline xfs_fileoff_t xfs_refcount_max_unmap(int log_res)

{

	return (log_res * 3 / 4) / XFS_REFCOUNT_ITEM_OVERHEAD;

}



#endif	/* __XFS_REFCOUNT_H__ */

fs/xfs/xfs_bmap_item.c

+15 −2
Original line number Diff line number Diff line
@@ -396,6 +396,7 @@ xfs_bui_recover( 
	struct xfs_map_extent		*bmap;

	xfs_fsblock_t			startblock_fsb;

	xfs_fsblock_t			inode_fsb;

	xfs_filblks_t			count;

	bool				op_ok;

	struct xfs_bud_log_item		*budp;

	enum xfs_bmap_intent_type	type;
@@ -404,6 +405,7 @@ xfs_bui_recover( 
	struct xfs_trans		*tp;

	struct xfs_inode		*ip = NULL;

	struct xfs_defer_ops		dfops;

	struct xfs_bmbt_irec		irec;

	xfs_fsblock_t			firstfsb;



	ASSERT(!test_bit(XFS_BUI_RECOVERED, &buip->bui_flags));
@@ -481,13 +483,24 @@ xfs_bui_recover( 
	}

	xfs_trans_ijoin(tp, ip, 0);



	count = bmap->me_len;

	error = xfs_trans_log_finish_bmap_update(tp, budp, &dfops, type,

			ip, whichfork, bmap->me_startoff,

			bmap->me_startblock, bmap->me_len,

			state);

			bmap->me_startblock, &count, state);

	if (error)

		goto err_dfops;



	if (count > 0) {

		ASSERT(type == XFS_BMAP_UNMAP);

		irec.br_startblock = bmap->me_startblock;

		irec.br_blockcount = count;

		irec.br_startoff = bmap->me_startoff;

		irec.br_state = state;

		error = xfs_bmap_unmap_extent(tp->t_mountp, &dfops, ip, &irec);

		if (error)

			goto err_dfops;

	}



	/* Finish transaction, free inodes. */

	error = xfs_defer_finish(&tp, &dfops, NULL);

	if (error)

CRA Git | Maintained and supported by SUSTech CRA and CCSE