On Wed, Nov 22, 2017 at 10:01:37PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> In xfs_ifree, we reset the data/attr forks to extents format without
> bothering to free any inline data buffer that might still be around
> after all the blocks have been truncated off the file. Prior to commit
> 43518812d2 ("xfs: remove support for inlining data/extents into the
> inode fork") nobody noticed because the leftover inline data after
> truncation was small enough to fit inside the inline buffer inside the
> fork itself.
>
> However, now that we've removed the inline buffer, we /always/ have to
> free the inline data buffer or else we leak them like crazy. This test
> was found by turning on kmemleak for generic/001 or generic/388.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_inode.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 61d1cb7..8012741 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2401,6 +2401,24 @@ xfs_ifree_cluster(
> }
>
> /*
> + * Free any local-format buffers sitting around before we reset to
> + * extents format.
> + */
> +static inline void
> +xfs_ifree_local_data(
> + struct xfs_inode *ip,
> + int whichfork)
> +{
> + struct xfs_ifork *ifp;
> +
> + if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> + return;
I'm new to all this so this was a bit hard to follow. I'm confused with how
commit 43518812d2 ("xfs: remove support for inlining data/extents into the
inode fork") exacerbated the leak, isn't that commit about
XFS_DINODE_FMT_EXTENTS?
Did we have cases where the format was XFS_DINODE_FMT_LOCAL and yet
ifp->if_u1.if_data == ifp->if_u2.if_inline_data ?
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
