On Thu, Jul 24, 2014 at 10:23:05AM -0400, Brian Foster wrote:
> An inode chunk is currently added to the transaction free list based on
> a simple fsb conversion and hardcoded chunk length. The nature of sparse
> chunks is such that the physical chunk of inodes on disk may consist of
> one or more discontiguous parts. Blocks that reside in the holes of the
> inode chunk are not inodes and could be allocated to any other use or
> not allocated at all.
>
> Refactor the existing xfs_bmap_add_free() call into the
> xfs_difree_inode_chunk() helper. The new helper uses the existing
> calculation if a chunk is not sparse. Otherwise, use the inobt record
> holemask to free the contiguous regions of the chunk.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_ialloc.c | 64 +++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 61 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index f75f191..1be57b1 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -1659,6 +1659,66 @@ out_error:
> return error;
> }
>
> +/*
> + * Free the blocks of an inode chunk. We must consider that the inode chunk
> + * might be sparse and only free the regions that are allocated as part of the
> + * chunk.
> + */
> +STATIC void
> +xfs_difree_inode_chunk(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + struct xfs_inobt_rec_incore *rec,
> + struct xfs_bmap_free *flist)
> +{
> + xfs_agblock_t sagbno = XFS_AGINO_TO_AGBNO(mp, rec->ir_startino);
> + xfs_agblock_t agbno;
> + int nextbit;
> + int contig, contigblk;
> + __uint16_t allocmask;
> + uint allocbitmap;
> +
> + if (!xfs_inobt_issparse(rec)) {
> + /* not sparse, calculate extent info directly */
> + xfs_bmap_add_free(XFS_AGB_TO_FSB(mp, agno,
> + XFS_AGINO_TO_AGBNO(mp, rec->ir_startino)),
> + mp->m_ialloc_blks, flist, mp);
> + return;
> + }
> +
> + /*
> + * The bit flip and type conversion are intentionally done separately
> + * here to zero-extend the bitmask.
> + */
> + allocmask = ~rec->ir_holemask;
> + allocbitmap = allocmask;
If we are going to keep this code, then a helper is absolutely
necessary....
> +
> + /*
> + * We now have an allocation bitmap in units of inodes at sparse chunk
> + * granularity (e.g., more than one inode per bit). Use the bitmask
> + * functions to find each contigious range of bits in the map. For each
> + * range, convert the start bit and count to block values and use that
> + * data to add the associated extent to the free list.
> + */
> + nextbit = xfs_next_bit(&allocbitmap, 1, 0);
> + while (nextbit != -1) {
> + agbno = (nextbit * XFS_INODES_PER_SPCHUNK) /
> + mp->m_sb.sb_inopblock;
> + agbno += sagbno;
> +
> + contig = xfs_contig_bits(&allocbitmap, 1, nextbit);
> + contigblk = (contig * XFS_INODES_PER_SPCHUNK) /
> + mp->m_sb.sb_inopblock;
> +
> + ASSERT(agbno % xfs_ialloc_cluster_alignment(mp) == 0);
> + ASSERT(contigblk % xfs_ialloc_cluster_alignment(mp) == 0);
> + xfs_bmap_add_free(XFS_AGB_TO_FSB(mp, agno, agbno), contigblk,
> + flist, mp);
> +
> + nextbit = xfs_next_bit(&allocbitmap, 1, nextbit + contig + 1);
> + }
Again, I think that the generic bitmap code is a better way to
implement this...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
