On Wed, 2011-08-24 at 02:04 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-simplify-xfs_bmap_isaeof)
> Add a common helper for finding the last extent in a file.
>
> Largely based on a patch from Dave Chinner.
The new version of xfs_bmap_isaeof() no longer asserts
that the data fork is the one being operated on. Why?
I have a couple of suggested comment clarifications,
and a suggested logic simplification below.
Reviewed-by: Alex Elder <aelder@xxxxxxx>
(Still reviewing the rest, but thought I'd fire off
a batch of the ones that I'm done with.)
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>
> Index: xfs/fs/xfs/xfs_bmap.c
. . .
> +
> +/*
> + * Check the last inode extent to determine whether this allocation will result
> + * in blocks being allocated at the end of the file. When we allocate new data
> + * blocks at the end of the file which do not start at the previous data block,
> + * we will try to align the new blocks at stripe unit boundaries.
It is not obvious what value will be returned when the
fork is empty, so mention that case in the comment header:
Returns 0 in *aeof if the file (fork) is empty.
(Maybe you could explain why this is true at a more
abstract level though.)
> + */
> +STATIC int
> +xfs_bmap_isaeof(
> + struct xfs_inode *ip,
> + xfs_fileoff_t off,
> + int whichfork,
> + char *aeof)
> +{
> + struct xfs_bmbt_irec rec;
> + int is_empty;
> + int error;
> +
> + *aeof = 0;
> + error = xfs_bmap_last_extent(NULL, ip, whichfork, &rec, &is_empty);
> + if (error || is_empty)
> + return error;
> +
> + /*
> + * Check we are allocating in the last extent (for delayed allocations)
> + * or past the last extent for non-delayed allocations.
> + */
> + *aeof = (off >= rec.br_startoff &&
> + off < rec.br_startoff + rec.br_blockcount &&
> + isnullstartblock(rec.br_startblock)) ||
> + off >= rec.br_startoff + rec.br_blockcount;
This logic could be shortened:
*aeof = off >= rec.br_startoff + rec.br_blockcount ||
(off >= rec.br_startoff &&
isnullstartblock(rec.br_startblock);
> + return 0;
> +}
> +
> +/*
> + * Check if the endoff is outside the last extent. If so the caller will grow
> + * the allocation to a stripe unit boundary.
All offsets are considered outside the end of file for an
empty file (fork), so 1 is returned in *eof in that case.
> + */
> +int
> +xfs_bmap_eof(
> + struct xfs_inode *ip,
> + xfs_fileoff_t endoff,
> + int whichfork,
> + int *eof)
> +{
> + struct xfs_bmbt_irec rec;
> + int error;
> +
> + error = xfs_bmap_last_extent(NULL, ip, whichfork, &rec, eof);
> + if (error || *eof)
> + return error;
> +
> + *eof = endoff >= rec.br_startoff + rec.br_blockcount;
> + return 0;
> +}
> +
. . .
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
