Re: [PATCH 4/3] xfs: don't assert on delalloc regions beyond EOF

Ben Myers
2012-05-08T17:22:27Z

On Sun, Apr 29, 2012 at 09:16:17PM +1000, Dave Chinner wrote:
>
> From: Dave Chinner <dchinner@redhat.com>
>
> When we are doing speculative delayed allocation beyond EOF,
> conversion of the region allocated beyond EOF is dependent on the
> largest free space extent available. If the largest free extent is
> smaller than the delalloc range, then after allocation we leave
> a delalloc extent that starts beyond EOF. This extent cannot *ever*
> be converted by flushing data, and so will remain there until either
> the EOF moves into the extent or it is truncated away.
>
> Hence if xfs_getbmap() runs on such an inode and is asked to return
> extents beyond EOF, it will assert fail on this extent even though
> there is nothing xfs_getbmap() can do to convert it to a real
> extent. Hence we should simply report these delalloc extents rather
> than assert that there should be none.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_bmap.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
> index 26ab256..478bce9 100644
> --- a/fs/xfs/xfs_bmap.c
> +++ b/fs/xfs/xfs_bmap.c
> @@ -5620,8 +5620,20 @@ xfs_getbmap(
> XFS_FSB_TO_BB(mp, map[i].br_blockcount);
> out[cur_ext].bmv_unused1 = 0;
> out[cur_ext].bmv_unused2 = 0;
> - ASSERT(((iflags & BMV_IF_DELALLOC) != 0) ||
> - (map[i].br_startblock != DELAYSTARTBLOCK));
> +
> + /*
> + * delayed allocation extents that start beyond EOF can
> + * occur due to speculative EOF allocation when the
> + * delalloc extent is larger than the largest freespace
> + * extent at conversion time. These extents cannot be
> + * converted by data writeback, so can exist here even
> + * if we are not supposed to be finding delalloc
> + * extents.
> + */
> + if (map[i].br_startblock == DELAYSTARTBLOCK &&
> + map[i].br_startoff <= XFS_B_TO_FSB(mp, XFS_ISIZE(ip)))
> + ASSERT((iflags & BMV_IF_DELALLOC) != 0);
> +

Looks fine. This assert will no longer kick off for delay extents after eof,
but will still catch any within the file.

Reviewed-by: Ben Myers <bpm@sgi.com>