From: Darrick J. Wong Subject: [PATCH] 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. Signed-off-by: Darrick J. Wong --- fs/xfs/libxfs/xfs_bmap.c | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 179b483..87da58d 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -5417,6 +5417,28 @@ xfs_bmap_del_extent( } /* + * Guesstimate how many blocks we can unmap without running the risk of + * blowing out the transaction with EFIs. + */ +static xfs_fileoff_t +xfs_bunmapi_can_unmap( + struct xfs_trans *tp, + struct xfs_inode *ip, + int whichfork, + xfs_fileoff_t len) +{ + unsigned int limit; + + if (!xfs_is_reflink_inode(ip) || whichfork != XFS_DATA_FORK) + return len; + + limit = (tp->t_log_res * 3 / 4) / 32; + if (len <= limit) + return len; + return limit; +} + +/* * Unmap (remove) blocks from a file. * If nexts is nonzero then the number of extents to remove is limited to * that value. If not all extents in the block range can be removed then @@ -5451,6 +5473,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 can_unmap; trace_xfs_bunmap(ip, bno, len, flags, _RET_IP_); @@ -5472,6 +5495,12 @@ __xfs_bunmapi( ASSERT(len > 0); ASSERT(nexts >= 0); + /* + * If this is potentially reflinked data blocks, don't blow out + * the reservation. + */ + can_unmap = xfs_bunmapi_can_unmap(tp, ip, whichfork, len); + if (!(ifp->if_flags & XFS_IFEXTENTS) && (error = xfs_iread_extents(tp, ip, whichfork))) return error; @@ -5516,7 +5545,7 @@ __xfs_bunmapi( extno = 0; while (bno != (xfs_fileoff_t)-1 && bno >= start && lastx >= 0 && - (nexts == 0 || extno < nexts)) { + (nexts == 0 || extno < nexts) && can_unmap > 0) { /* * Is the found extent after a hole in which bno lives? * Just back up to the previous extent, if so. @@ -5548,6 +5577,16 @@ __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 (can_unmap < del.br_blockcount) { +printk(KERN_ERR "%s: ino %lld pblk %d:%llu lblk %llu len %llu can_unmap %llu got=%llu:%llu:%llu\n", __func__, ip->i_ino, wasdel, del.br_startblock, del.br_startoff, del.br_blockcount, can_unmap, got.br_startblock, got.br_startoff, got.br_blockcount); + del.br_startoff += del.br_blockcount - can_unmap; + if (!wasdel) + del.br_startblock += del.br_blockcount - can_unmap; + del.br_blockcount = can_unmap; + } + sum = del.br_startblock + del.br_blockcount; if (isrt && (mod = do_mod(sum, mp->m_sb.sb_rextsize))) { @@ -5724,6 +5763,7 @@ __xfs_bunmapi( if (!isrt && wasdel) xfs_mod_fdblocks(mp, (int64_t)del.br_blockcount, false); + can_unmap -= del.br_blockcount; bno = del.br_startoff - 1; nodelete: /*