Re: [PATCH 2/3] xfs: push down inactive transaction mgmt for truncate

Dave Chinner
2013-09-18T23:00:14Z

On Wed, Sep 18, 2013 at 12:15:59PM -0400, Brian Foster wrote:
> Create the new xfs_inactive_truncate() function to handle the
> truncate portion of xfs_inactive(). Push the locking and
> transaction management into the new function.
>
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
> fs/xfs/xfs_inode.c | 112 ++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 63 insertions(+), 49 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 30db70e..9416462 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1663,6 +1663,53 @@ xfs_release(
> }
>
> /*
> + * xfs_inactive_truncate
> + *
> + * Called to perform a truncate when an inode becomes unlinked.
> + */
> +STATIC int
> +xfs_inactive_truncate(
> + struct xfs_inode *ip)
> +{
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_trans *tp;
> + int error;
> +
> + tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
> +
> + error = xfs_trans_reserve(tp, &M_RES(mp)->tr_itruncate, 0, 0);
> + if (error) {
> + ASSERT(XFS_FORCED_SHUTDOWN(mp));
> + goto error0;

error_trans_cancel...

> + }
> +
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, ip, 0);
> +
> + ip->i_d.di_size = 0;
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);

Can you add a comment here that we are logging the inode size so
that if the system crashes part way through the truncation we don't
need to worry about stale data exposure? There's a similar, more
expansive comment in xfs_setattr_size() - maybe a quick one-line
explaination and a "see setattr_size for more info" woul dbe
sufficient.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com