On 12/13 2013 06:18 AM, Dave Chinner wrote:
> 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....
Yup, I have a separate patch for doing that in my current quota check working branch.
Originally, I also did it as the 1st patch in RFC parallel quota check:
http://www.spinics.net/lists/xfs/msg23618.html
But looks I should isolate it at xfs_itable.c if no other users for now.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
