Re: [PATCH v2 5/8] xfs: use xfs_icluster_size_fsb in xfs_bulkstat

[Dave Chinner <david@fromorbit.com>]

On Thu, Dec 12, 2013 at 03:38:40PM +0800, Jeff Liu wrote:
> From: Jie Liu <jeff.liu@oracle.com>
> 
> Use xfs_icluster_size_fsb() in xfs_bulkstat(), make the related
> variables more meaningful and remove an unused variable nimask
> from it.
> 
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>

Looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

At some point we need to factor this code a bit to get rid of the
levels of indenting it has.....

> @@ -390,12 +386,12 @@ xfs_bulkstat(
>  				agbno = XFS_AGINO_TO_AGBNO(mp, r.ir_startino);
>  				for (chunkidx = 0;
>  				     chunkidx < XFS_INODES_PER_CHUNK;
> -				     chunkidx += nicluster,
> -				     agbno += nbcluster) {
> -					if (xfs_inobt_maskn(chunkidx, nicluster)
> -							& ~r.ir_free)
> +				     chunkidx += inodes_per_cluster,
> +				     agbno += blks_per_cluster) {
> +					if (xfs_inobt_maskn(chunkidx,
> +					    inodes_per_cluster) & ~r.ir_free)
>  						xfs_btree_reada_bufs(mp, agno,
> -							agbno, nbcluster,
> +							agbno, blks_per_cluster,
>  							&xfs_inode_buf_ops);
>  				}
>  				blk_finish_plug(&plug);

e.g. this readahead loop could be factored into

static void
xfs_ichunk_ra(
	*mp, *rec, inodes_per_cluster, blks_per_cluster)
{
	blk_start_plug(&plug);
	agbno = XFS_AGINO_TO_AGBNO(mp, r.ir_startino);
	for (chunkidx = 0; chunkidx < XFS_INODES_PER_CHUNK;
	     chunkidx += inodes_per_cluster, agbno += blks_per_cluster) {
		if (xfs_inobt_maskn(chunkidx, inodes_per_cluster) & ~r.ir_free)
			xfs_btree_reada_bufs(mp, agno, agbno, blks_per_cluster,
					     &xfs_inode_buf_ops);
	}
	blk_finish_plug(&plug);
}

Doing this to all the separate parts of the bulkstat code would make
it an awful lot easier to read and modify in future....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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