On Fri, Sep 20, 2013 at 11:06:09AM -0400, Brian Foster wrote:
> Push down the transaction management for remote symlinks from
> xfs_inactive() down to xfs_inactive_symlink_rmt(). The latter is
> cleaned up to avoid transaction management intended for the
> calling context (i.e., trans duplication, reservation, item
> attachment).
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
Looks good. One minor quibble if you need to respin the patches
again, but otherwise:
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
> @@ -563,41 +552,46 @@ xfs_inactive_symlink_rmt(
> */
> int
> xfs_inactive_symlink(
> - struct xfs_inode *ip,
> - struct xfs_trans **tp)
> + struct xfs_inode *ip)
> {
> struct xfs_mount *mp = ip->i_mount;
> int pathlen;
>
> trace_xfs_inactive_symlink(ip);
>
> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> -
> if (XFS_FORCED_SHUTDOWN(mp))
> return XFS_ERROR(EIO);
>
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> +
> /*
> * Zero length symlinks _can_ exist.
> */
> pathlen = (int)ip->i_d.di_size;
> - if (!pathlen)
> + if (!pathlen) {
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> return 0;
> + }
Minor quibble: this repeated "unlock, return error" pattern could
be done with:
....
if (!pathlen)
goto out_unlock;
....
/* remove the remote symlink */
return xfs_inactive_symlink_rmt(ip);
out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
}
But, like I said, there's no need to do this unless I find other
things in the rest of the series that require a respin...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
