On Thu, Sep 19, 2013 at 03:15:19PM -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@xxxxxxxxxx>
> ---
> fs/xfs/xfs_inode.c | 117 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 68 insertions(+), 49 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 30db70e..33bb9be 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1663,6 +1663,58 @@ 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 error_trans_cancel;
> + }
> +
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, ip, 0);
> +
> + /*
> + * Log the inode size first to prevent stale data exposure in the event
> + * of a system crash before the truncate completes. See the related
> + * comment in xfs_setattr_size() for details.
> + */
> + ip->i_d.di_size = 0;
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> +
> + error = xfs_itruncate_extents(&tp, ip, XFS_DATA_FORK, 0);
> + if (error)
> + goto error_unlock;
> +
> + ASSERT(ip->i_d.di_nextents == 0);
> +
> + error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
> + if (error)
> + goto error_unlock;
Same again - no cancel on commit error, just fall through and return
the error....
> +
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> + return 0;
> +
> +error_unlock:
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +error_trans_cancel:
> + xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
Same again w.r.t. unlock/cancel. FWIW, see xfs_free_file_space() for
an example of how the ordering is supposed to work.
And, note:
> - tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
> - resp = truncate ? &M_RES(mp)->tr_itruncate : &M_RES(mp)->tr_ifree;
> -
> - error = xfs_trans_reserve(tp, resp, 0, 0);
> - if (error) {
> - ASSERT(XFS_FORCED_SHUTDOWN(mp));
> - xfs_trans_cancel(tp, 0);
Different flags :)
> @@ -1831,13 +1854,9 @@ xfs_inactive(
> * Release the dquots held by inode, if any.
> */
> xfs_qm_dqdetach(ip);
> -out_unlock:
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> out:
> return VN_INACTIVE_CACHE;
> -out_cancel:
> - xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
> - goto out_unlock;
And the ordering of the previous code... :)
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
