On Mon, Oct 08, 2012 at 10:57:16PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> When updating new secondary superblocks in a growfs operation, the
> sueprblock buffer is read from the newly grown region of the
> underlying device. This is not guaranteed to be zero, so violates
> the underlying assumption that the unused parts of superblocks are
> zero filled. Get a new buffer for these secondary superblocks to
> ensure that the unused regions are zero filled correctly.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_fsops.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index c25b094..4beaede 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -399,9 +399,26 @@ xfs_growfs_data_private(
>
> /* update secondary superblocks. */
> for (agno = 1; agno < nagcount; agno++) {
> - error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
> + error = 0;
> + /*
> + * new secondary superblocks need to be zeroed, not read from
> + * disk as the contents of the new area we are growing into is
> + * completely unknown.
> + */
> + if (agno < oagcount) {
> + error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
> XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> XFS_FSS_TO_BB(mp, 1), 0, &bp);
> + } else {
> + bp = xfs_trans_get_buf(NULL, mp->m_ddev_targp,
> + XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
> + XFS_FSS_TO_BB(mp, 1), 0);
> + if (bp)
> + xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
> + else
> + error = ENOMEM;
> + }
> +
> if (error) {
> xfs_warn(mp,
> "error %d reading secondary superblock for ag %d",
> @@ -423,7 +440,7 @@ xfs_growfs_data_private(
> break; /* no point in continuing */
> }
> }
> - return 0;
> + return error;
>
> error0:
> xfs_trans_cancel(tp, XFS_TRANS_ABORT);
> --
> 1.7.10
>
Looks good.
Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
--
--Carlos
|