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

[Dave Chinner <david@xxxxxxxxxxxxx>]

On Thu, Dec 12, 2013 at 03:38:40PM +0800, Jeff Liu wrote:
> From: Jie Liu <jeff.liu@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>

Looks fine.

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>

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@xxxxxxxxxxxxx