On Thu, Apr 24, 2014 at 03:32:59PM +1000, Dave Chinner wrote: > On Thu, Apr 10, 2014 at 12:08:29PM -0400, Brian Foster wrote: > > Create the xfs_calc_finobt_res() helper to calculate the finobt log > > reservation for inode allocation and free. Update > > XFS_IALLOC_SPACE_RES() to reserve blocks for the additional finobt > > insertion on inode allocation. Create XFS_IFREE_SPACE_RES() to > > reserve blocks for the potential finobt record insertion on inode > > free (i.e., if an inode chunk was previously fully allocated). > > > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx> > > --- > > fs/xfs/xfs_inode.c | 25 ++++++++++++++++++++++- > > fs/xfs/xfs_trans_resv.c | 53 +++++++++++++++++++++++++++++++++++++++++++++--- > > fs/xfs/xfs_trans_space.h | 7 ++++++- > > 3 files changed, 80 insertions(+), 5 deletions(-) > > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > > index 5e7a38f..8576892 100644 > > --- a/fs/xfs/xfs_inode.c > > +++ b/fs/xfs/xfs_inode.c > > @@ -1837,8 +1837,31 @@ xfs_inactive_ifree( > > int error; > > > > tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE); > > - error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ifree, 0, 0); > > + > > + /* > > + * The ifree transaction might need to allocate blocks for record > > + * insertion to the finobt. We don't want to fail here at ENOSPC, so > > + * allow ifree to dip into the reserved block pool if necessary. > > + * > > + * Freeing large sets of inodes generally means freeing inode chunks, > > + * directory and file data blocks, so this should be relatively safe. > > + * Only under severe circumstances should it be possible to free enough > > + * inodes to exhaust the reserve block pool via finobt expansion while > > + * at the same time not creating free space in the filesystem. > > + * > > + * Send a warning if the reservation does happen to fail, as the inode > > + * now remains allocated and sits on the unlinked list until the fs is > > + * repaired. > > + */ > > + tp->t_flags |= XFS_TRANS_RESERVE; > > + error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ifree, > > + XFS_IFREE_SPACE_RES(mp), 0); > > if (error) { > > + if (error == ENOSPC) > > + xfs_warn_ratelimited(mp, > > + "Failed to remove inode(s) from unlinked list. " > > + "Please free space, unmount and run xfs_repair."); > > + > > ASSERT(XFS_FORCED_SHUTDOWN(mp)); > > xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES); > > return error; > > Comment is good, but if we get an ENOSPC error here we won't be in a > shutdown state, hence the assert needs fixing (i.e. else {assert}) > Yeah, good catch. Brian > Otherwise this looks good. > > Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs