On Mon, Feb 09, 2015 at 10:02:32AM +1100, Dave Chinner wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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.
>
Ok.
> > 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...
>
Hmm, yeah I added that blks comparison recently simply to prevent
another allocation attempt on such systems where the block size is >=
the chunk size.
This changes behavior of such systems either way, but debug mode has
that effect regardless I suppose. We hit a codepath that we wouldn't
normally hit in that large bsize configuration, but it's debug mode and
the alignment/size of the allocation should be equivalent to the
original allocation code so I think it should be fine to move the label.
Brian
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
