On Mon, Nov 14, 2016 at 06:12:33PM +0100, Christoph Hellwig wrote:
> Rewrite the function using xfs_iext_lookup_extent and xfs_iext_get_extent,
> and massage the flow into something easily understandable.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 64 ++++++++++++++++++++++++------------------------
> 1 file changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 5c3c4dd..98f490b 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1523,44 +1523,44 @@ xfs_bmap_first_unused(
> */
> int /* error */
> xfs_bmap_last_before(
> - xfs_trans_t *tp, /* transaction pointer */
> - xfs_inode_t *ip, /* incore inode */
> - xfs_fileoff_t *last_block, /* last block */
> - int whichfork) /* data or attr fork */
> + struct xfs_trans *tp, /* transaction pointer */
> + struct xfs_inode *ip, /* incore inode */
> + xfs_fileoff_t *last_block, /* last block */
> + int whichfork) /* data or attr fork */
> {
> - xfs_fileoff_t bno; /* input file offset */
> - int eof; /* hit end of file */
> - xfs_bmbt_rec_host_t *ep; /* pointer to last extent */
> - int error; /* error return value */
> - xfs_bmbt_irec_t got; /* current extent value */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> - xfs_extnum_t lastx; /* last extent used */
> - xfs_bmbt_irec_t prev; /* previous extent value */
> + struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> + struct xfs_bmbt_irec got;
> + xfs_extnum_t idx;
> + int error;
>
> - if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
> - XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
> - XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> - return -EIO;
> - if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
> + switch (XFS_IFORK_FORMAT(ip, whichfork)) {
> + case XFS_DINODE_FMT_LOCAL:
> *last_block = 0;
> return 0;
> + case XFS_DINODE_FMT_BTREE:
> + case XFS_DINODE_FMT_EXTENTS:
> + break;
> + default:
> + return -EIO;
> }
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> - if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> - (error = xfs_iread_extents(tp, ip, whichfork)))
> - return error;
> - bno = *last_block - 1;
> - ep = xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got,
> - &prev);
> - if (eof || xfs_bmbt_get_startoff(ep) > bno) {
> - if (prev.br_startoff == NULLFILEOFF)
> - *last_block = 0;
> - else
> - *last_block = prev.br_startoff + prev.br_blockcount;
> +
> + if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> + error = xfs_iread_extents(tp, ip, whichfork);
> + if (error)
> + return error;
> }
> - /*
> - * Otherwise *last_block is already the right answer.
> - */
> +
> + if (xfs_iext_lookup_extent(ip, ifp, *last_block - 1, &idx, &got)) {
> + if (got.br_startoff <= *last_block - 1)
> + return 0;
> + }
> +
> + if (xfs_iext_get_extent(ifp, idx - 1, &got)) {
It may not be an issue in current usage, but I think we should check for
idx here (i.e., 'if (idx && xfs_iext_get_extent(..))').
Brian
> + *last_block = got.br_startoff + got.br_blockcount;
> + return 0;
> + }
> +
> + *last_block = 0;
> return 0;
> }
>
> --
> 2.1.4
>
> --
> 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
--
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
