* [PATCH v2 5/8] xfs: use xfs_icluster_size_fsb in xfs_bulkstat
@ 2013-12-12 7:38 Jeff Liu
2013-12-12 22:18 ` Dave Chinner
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Liu @ 2013-12-12 7:38 UTC (permalink / raw)
To: xfs@oss.sgi.com
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>
---
fs/xfs/xfs_itable.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 0571012..f463382 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -209,9 +209,8 @@ xfs_bulkstat(
xfs_inobt_rec_incore_t *irbuf; /* start of irec buffer */
xfs_inobt_rec_incore_t *irbufend; /* end of good irec buffer entries */
xfs_ino_t lastino; /* last inode number returned */
- int nbcluster; /* # of blocks in a cluster */
- int nicluster; /* # of inodes in a cluster */
- int nimask; /* mask for inode clusters */
+ int blks_per_cluster; /* # of blocks per cluster */
+ int inodes_per_cluster;/* # of inodes per cluster */
int nirbuf; /* size of irbuf */
int rval; /* return value error code */
int tmp; /* result value from btree calls */
@@ -243,11 +242,8 @@ xfs_bulkstat(
*done = 0;
fmterror = 0;
ubufp = ubuffer;
- nicluster = mp->m_sb.sb_blocksize >= mp->m_inode_cluster_size ?
- mp->m_sb.sb_inopblock :
- (mp->m_inode_cluster_size >> mp->m_sb.sb_inodelog);
- nimask = ~(nicluster - 1);
- nbcluster = nicluster >> mp->m_sb.sb_inopblog;
+ blks_per_cluster = xfs_icluster_size_fsb(mp);
+ inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
irbuf = kmem_zalloc_greedy(&irbsize, PAGE_SIZE, PAGE_SIZE * 4);
if (!irbuf)
return ENOMEM;
@@ -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);
--
1.8.3.2
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2 5/8] xfs: use xfs_icluster_size_fsb in xfs_bulkstat
2013-12-12 7:38 [PATCH v2 5/8] xfs: use xfs_icluster_size_fsb in xfs_bulkstat Jeff Liu
@ 2013-12-12 22:18 ` Dave Chinner
2013-12-13 2:20 ` Jeff Liu
0 siblings, 1 reply; 3+ messages in thread
From: Dave Chinner @ 2013-12-12 22:18 UTC (permalink / raw)
To: Jeff Liu; +Cc: xfs@oss.sgi.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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2 5/8] xfs: use xfs_icluster_size_fsb in xfs_bulkstat
2013-12-12 22:18 ` Dave Chinner
@ 2013-12-13 2:20 ` Jeff Liu
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Liu @ 2013-12-13 2:20 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs@oss.sgi.com
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@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....
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@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-12-13 2:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-12 7:38 [PATCH v2 5/8] xfs: use xfs_icluster_size_fsb in xfs_bulkstat Jeff Liu
2013-12-12 22:18 ` Dave Chinner
2013-12-13 2:20 ` Jeff Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox