xfs
[Top] [All Lists]

Re: [PATCH 2/5] xfs: use xfs_ialloc_blks_per_cluster in xfs_bulkstat

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/5] xfs: use xfs_ialloc_blks_per_cluster in xfs_bulkstat
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Thu, 12 Dec 2013 11:00:40 +0800
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131211232741.GO10988@dastard>
References: <52A86D1E.6090507@xxxxxxxxxx> <20131211232741.GO10988@dastard>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0
On 12/12 2013 07:27 AM, Dave Chinner wrote:
> On Wed, Dec 11, 2013 at 09:48:14PM +0800, Jeff Liu wrote:
>> From: Jie Liu <jeff.liu@xxxxxxxxxx>
>>
>> Use xfs_ialloc_blks_per_cluster() 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>
>> ---
>>  fs/xfs/xfs_itable.c | 20 ++++++++------------
>>  1 file changed, 8 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
>> index c237ad1..6671f5b 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 in a cluster */
>> +    int                     ninodes; /* # of inodes in a cluster */
> 
> inodes_per_cluster?
Ok.
> 
>>      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 >= XFS_INODE_CLUSTER_SIZE(mp) ?
>> -            mp->m_sb.sb_inopblock :
>> -            (XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog);
>> -    nimask = ~(nicluster - 1);
>> -    nbcluster = nicluster >> mp->m_sb.sb_inopblog;
>> +    blks_per_cluster = xfs_ialloc_blks_per_cluster(mp);
> 
> Ok, now I see the helper name, perhaps it shoul dbe more like all
> the other conversion functions. "ialloc" means nothing in the
> context, and "blks" doesn't tell us the unit we are converting to
> - if could be basic blocks, it could be filesystem blocks. So,
> perhaps something like xfs_icluster_size_fsb()?
Sounds make sense to me, I'd like to adopt this name.

Thanks,
-Jeff

<Prev in Thread] Current Thread [Next in Thread>