xfs
[Top] [All Lists]

Re: [RFC PATCH 1/4] xfs: introduce a new helper xfs_inobt_reada_chunk()

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [RFC PATCH 1/4] xfs: introduce a new helper xfs_inobt_reada_chunk()
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Mon, 18 Nov 2013 20:21:26 +0800
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131118110113.GA1304@xxxxxxxxxxxxx>
References: <5281F509.7020105@xxxxxxxxxx> <20131115170325.GA16942@xxxxxxxxxxxxx> <5288B58D.5030609@xxxxxxxxxx> <20131118110113.GA1304@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120410 Thunderbird/11.0.1
On 11/18 2013 07:01 PM, Christoph Hellwig wrote:

>>> I'd prefer to factor this out even further. xfs_ialloc_inode_init and
>>> xfs_ifree_cluster already have two pieces of code that calculate these
>>> two (with more readable names) and an additional nuber buffers counter
>>> we won't need here, it might make most sense to factor that into a
>>> single common helper.
>> Yup, I also thought this can be factored out, however, I can not figure out
>> a meaningful function name at that time due to my poor skill...
>>
>> How about if we introduce an inline helper to xfs_ialloc.h as below?
>>
>> /* Helper function to extract the # of blocks/inodes/buffers hint per 
>> cluster */
>> static inline void
>> xfs_ialloc_get_cluster_hints(
>>      struct xfs_mount        *mp,
>>      int                     *nblks;
>>      int                     *ninodes;
>>      int                     *nbufs)
>> {
>>      ....
>> }
> 
> Probably fine to make it an inline.  I don't think we need the nbufs
> parameter, as it requires the length to calculate, and it's a trivial
> length / blks_per_cluster.
> 
> Similarly the ninodes value is trivially calculatable, so it might be
> as easy as:
> 
> static inline int
> xfs_ialloc_blks_per_cluster(struct xfs_mount *mp)
> {
>       if (mp->m_sb.sb_blocksize >= XFS_INODE_CLUSTER_SIZE(mp))
>               return 1;
>       return XFS_INODE_CLUSTER_SIZE(mp) / mp->m_sb.sb_blocksize;
> }
> 
> 
> ...
> 
>       blks_per_cluster = xfs_ialloc_blks_per_cluster(mp);
>       nbufs = length / blks_per_cluster;
>       ninodes = blks_per_cluster * mp->m_sb.sb_inopblock;

Coool, your idea is better than mine.

Thanks,
-Jeff


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