Re: [PATCH 1/2] xfs: fix realtime bitmap/summary file truncation when growing rt volume

[Chandan Babu R <chandanrlinux@gmail.com>]

On Thursday 8 October 2020 9:26:06 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The realtime bitmap and summary files are regular files that are hidden
> away from the directory tree.  Since they're regular files, inode
> inactivation will try to purge what it thinks are speculative
> preallocations beyond the incore size of the file.  Unfortunately,
> xfs_growfs_rt forgets to update the incore size when it resizes the
> inodes, with the result that inactivating the rt inodes at unmount time
> will cause their contents to be truncated.
> 
> Fix this by updating the incore size when we change the ondisk size as
> part of updating the superblock.  Note that we don't do this when we're
> allocating blocks to the rt inodes because we actually want those blocks
> to get purged if the growfs fails.
> 
> This fixes corruption complaints from the online rtsummary checker when
> running xfs/233.  Since that test requires rmap, one can also trigger
> this by growing an rt volume, cycling the mount, and creating rt files.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_rtalloc.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
>

Looks good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 9d4e33d70d2a..1c3969807fb9 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1027,10 +1027,13 @@ xfs_growfs_rt(
>  		xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL);
>  		xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL);
>  		/*
> -		 * Update the bitmap inode's size.
> +		 * Update the bitmap inode's size ondisk and incore.  We need
> +		 * to update the incore size so that inode inactivation won't
> +		 * punch what it thinks are "posteof" blocks.
>  		 */
>  		mp->m_rbmip->i_d.di_size =
>  			nsbp->sb_rbmblocks * nsbp->sb_blocksize;
> +		i_size_write(VFS_I(mp->m_rbmip), mp->m_rbmip->i_d.di_size);
>  		xfs_trans_log_inode(tp, mp->m_rbmip, XFS_ILOG_CORE);
>  		/*
>  		 * Get the summary inode into the transaction.
> @@ -1038,9 +1041,12 @@ xfs_growfs_rt(
>  		xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL);
>  		xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL);
>  		/*
> -		 * Update the summary inode's size.
> +		 * Update the summary inode's size.  We need to update the
> +		 * incore size so that inode inactivation won't punch what it
> +		 * thinks are "posteof" blocks.
>  		 */
>  		mp->m_rsumip->i_d.di_size = nmp->m_rsumsize;
> +		i_size_write(VFS_I(mp->m_rsumip), mp->m_rsumip->i_d.di_size);
>  		xfs_trans_log_inode(tp, mp->m_rsumip, XFS_ILOG_CORE);
>  		/*
>  		 * Copy summary data from old to new sizes.
> 
> 


-- 
chandan