Re: [PATCH v3 11/18] xfs: randomly do sparse inode allocations in DEBUG mode

[Dave Chinner <david@fromorbit.com>]

On Fri, Feb 06, 2015 at 02:52:58PM -0500, Brian Foster wrote:
> Sparse inode allocations generally only occur when full inode chunk
> allocation fails. This requires some level of filesystem space usage and
> fragmentation.
> 
> For filesystems formatted with sparse inode chunks enabled, do random
> sparse inode chunk allocs when compiled in DEBUG mode to increase test
> coverage.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 090d114..3e5d3eb 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -652,9 +652,18 @@ xfs_ialloc_ag_alloc(
>  
>  	struct xfs_perag *pag;
>  
> +#ifdef DEBUG
> +	int		do_sparse = 0;
> +
> +	/* randomly do sparse inode allocations */
> +	if (xfs_sb_version_hassparseinodes(&tp->t_mountp->m_sb))
> +		do_sparse = prandom_u32() & 1;
> +#endif
> +

Bit ugly with all the ifdefs. If you define the do_sparse variable
outside the ifdef, then the rest of the code other than this check
doesn't need ifdefs.

>  	memset(&args, 0, sizeof(args));
>  	args.tp = tp;
>  	args.mp = tp->t_mountp;
> +	args.fsbno = NULLFSBLOCK;
>  
>  	/*
>  	 * Locking will ensure that we don't have two callers in here
> @@ -675,6 +684,10 @@ xfs_ialloc_ag_alloc(
>  	agno = be32_to_cpu(agi->agi_seqno);
>  	args.agbno = XFS_AGINO_TO_AGBNO(args.mp, newino) +
>  		     args.mp->m_ialloc_blks;
> +#ifdef DEBUG
> +	if (do_sparse)
> +		goto sparse_alloc;
> +#endif
>  	if (likely(newino != NULLAGINO &&
>  		  (args.agbno < be32_to_cpu(agi->agi_length)))) {
>  		args.fsbno = XFS_AGB_TO_FSB(args.mp, agno, args.agbno);
> @@ -713,8 +726,7 @@ xfs_ialloc_ag_alloc(
>  		 * subsequent requests.
>  		 */
>  		args.minalignslop = 0;
> -	} else
> -		args.fsbno = NULLFSBLOCK;
> +	}
>  
>  	if (unlikely(args.fsbno == NULLFSBLOCK)) {
>  		/*
> @@ -769,6 +781,9 @@ xfs_ialloc_ag_alloc(
>  	 * Finally, try a sparse allocation if the filesystem supports it and
>  	 * the sparse allocation length is smaller than a full chunk.
>  	 */
> +#ifdef DEBUG
> +sparse_alloc:
> +#endif
>  	if (xfs_sb_version_hassparseinodes(&args.mp->m_sb) &&
>  	    args.mp->m_ialloc_min_blks < args.mp->m_ialloc_blks &&
>  	    args.fsbno == NULLFSBLOCK) {

The label can go after the if() statement, right? We've already
guaranteed all those other parameters are true, though I suspect
there's a case where that m_ialloc_min_blks < m_ialloc_blks will
fail: block size larger than inode chunk size (e.g. 64k block size, 512
byte inodes) so that would result in the code above failing to
allocate any inode chunks at all...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs