Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 396503fc authored by Dave Chinner's avatar Dave Chinner Committed by Dave Chinner
Browse files

xfs: sanitise error handling in xfs_alloc_fix_freelist 



The error handling is currently an inconsistent mess as every error
condition handles return values and releasing buffers individually.
Clean this up by using gotos and a sane error label stack.

Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent 72d55285

fs/xfs/libxfs/xfs_alloc.c

+55 −56
Original line number Diff line number Diff line
@@ -1877,84 +1877,77 @@ xfs_alloc_space_available( 
 */

STATIC int			/* error */

xfs_alloc_fix_freelist(

	xfs_alloc_arg_t	*args,	/* allocation argument structure */

	struct xfs_alloc_arg	*args,	/* allocation argument structure */

	int			flags)	/* XFS_ALLOC_FLAG_... */

{

	xfs_buf_t	*agbp;	/* agf buffer pointer */

	xfs_buf_t	*agflbp;/* agfl buffer pointer */

	struct xfs_mount	*mp = args->mp;

	struct xfs_perag	*pag = args->pag;

	struct xfs_trans	*tp = args->tp;

	struct xfs_buf		*agbp = NULL;

	struct xfs_buf		*agflbp = NULL;

	struct xfs_alloc_arg	targs;	/* local allocation arguments */

	xfs_agblock_t		bno;	/* freelist block */

	int		error;	/* error result code */

	xfs_mount_t	*mp;	/* file system mount point structure */

	xfs_extlen_t		need;	/* total blocks needed in freelist */

	xfs_perag_t	*pag;	/* per-ag information structure */

	xfs_alloc_arg_t	targs;	/* local allocation arguments */

	xfs_trans_t	*tp;	/* transaction pointer */



	mp = args->mp;

	int			error;



	pag = args->pag;

	tp = args->tp;

	if (!pag->pagf_init) {

		if ((error = xfs_alloc_read_agf(mp, tp, args->agno, flags,

				&agbp)))

			return error;

		error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);

		if (error)

			goto out_no_agbp;

		if (!pag->pagf_init) {

			ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);

			ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));

			args->agbp = NULL;

			return 0;

			goto out_agbp_relse;

		}

	}

	} else

		agbp = NULL;



	/*

	 * If this is a metadata preferred pag and we are user data

	 * then try somewhere else if we are not being asked to

	 * try harder at this point

	 * If this is a metadata preferred pag and we are user data then try

	 * somewhere else if we are not being asked to try harder at this

	 * point

	 */

	if (pag->pagf_metadata && args->userdata &&

	    (flags & XFS_ALLOC_FLAG_TRYLOCK)) {

		ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));

		args->agbp = NULL;

		return 0;

		goto out_agbp_relse;

	}



	need = XFS_MIN_FREELIST_PAG(pag, mp);

	if (!xfs_alloc_space_available(args, need, flags)) {

		if (agbp)

			xfs_trans_brelse(tp, agbp);

		args->agbp = NULL;

		return 0;

	}

	if (!xfs_alloc_space_available(args, need, flags))

		goto out_agbp_relse;



	/*

	 * Get the a.g. freespace buffer.

	 * Can fail if we're not blocking on locks, and it's held.

	 */

	if (agbp == NULL) {

		if ((error = xfs_alloc_read_agf(mp, tp, args->agno, flags,

				&agbp)))

			return error;

		if (agbp == NULL) {

	if (!agbp) {

		error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);

		if (error)

			goto out_no_agbp;

		if (!agbp) {

			ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);

			ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));

			args->agbp = NULL;

			return 0;

			goto out_no_agbp;

		}

	}





	/* If there isn't enough total space or single-extent, reject it. */

	need = XFS_MIN_FREELIST_PAG(pag, mp);

	if (!xfs_alloc_space_available(args, need, flags)) {

		xfs_trans_brelse(tp, agbp);

		args->agbp = NULL;

		return 0;

	}

	if (!xfs_alloc_space_available(args, need, flags))

		goto out_agbp_relse;



	/*

	 * Make the freelist shorter if it's too long.

	 *

	 * Note that from this point onwards, we will always release the agf and

	 * agfl buffers on error. This handles the case where we error out and

	 * the buffers are clean or may not have been joined to the transaction

	 * and hence need to be released manually. If they have been joined to

	 * the transaction, then xfs_trans_brelse() will handle them

	 * appropriately based on the recursion count and dirty state of the

	 * buffer.

	 *

	 * XXX (dgc): When we have lots of free space, does this buy us

	 * anything other than extra overhead when we need to put more blocks

	 * back on the free list? Maybe we should only do this when space is
@@ -1965,10 +1958,10 @@ xfs_alloc_fix_freelist( 


		error = xfs_alloc_get_freelist(tp, agbp, &bno, 0);

		if (error)

			return error;

			goto out_agbp_relse;

		error = xfs_free_ag_extent(tp, agbp, args->agno, bno, 1, 1);

		if (error)

			return error;

			goto out_agbp_relse;

		bp = xfs_btree_get_bufs(mp, tp, args->agno, bno, 0);

		xfs_trans_binval(tp, bp);

	}
@@ -1983,7 +1976,7 @@ xfs_alloc_fix_freelist( 
	targs.pag = pag;

	error = xfs_alloc_read_agfl(mp, tp, targs.agno, &agflbp);

	if (error)

		return error;

		goto out_agbp_relse;



	/* Make the freelist longer if it's too short. */

	while (pag->pagf_flcount < need) {
@@ -1992,10 +1985,9 @@ xfs_alloc_fix_freelist( 


		/* Allocate as many blocks as possible at once. */

		error = xfs_alloc_ag_vextent(&targs);

		if (error) {

			xfs_trans_brelse(tp, agflbp);

			return error;

		}

		if (error)

			goto out_agflbp_relse;



		/*

		 * Stop if we run out.  Won't happen if callers are obeying

		 * the restrictions correctly.  Can happen for free calls
@@ -2004,9 +1996,7 @@ xfs_alloc_fix_freelist( 
		if (targs.agbno == NULLAGBLOCK) {

			if (flags & XFS_ALLOC_FLAG_FREEING)

				break;

			xfs_trans_brelse(tp, agflbp);

			args->agbp = NULL;

			return 0;

			goto out_agflbp_relse;

		}

		/*

		 * Put each allocated block on the list.
@@ -2015,12 +2005,21 @@ xfs_alloc_fix_freelist( 
			error = xfs_alloc_put_freelist(tp, agbp,

							agflbp, bno, 0);

			if (error)

				return error;

				goto out_agflbp_relse;

		}

	}

	xfs_trans_brelse(tp, agflbp);

	args->agbp = agbp;

	return 0;



out_agflbp_relse:

	xfs_trans_brelse(tp, agflbp);

out_agbp_relse:

	if (agbp)

		xfs_trans_brelse(tp, agbp);

out_no_agbp:

	args->agbp = NULL;

	return error;

}



/*