[PATCH 1/5] xfs: introduce xfs_ialloc_blks_per_cluster

[PATCH 1/5] xfs: introduce xfs_ialloc_blks_per_cluster

[Jeff Liu]

From: Jie Liu <jeff.liu@oracle.com>

Introduce a common routine xfs_ialloc_blks_per_cluster() to calculate
and return the number of blocks per inode cluster.

Signed-off-by: Jie Liu <jeff.liu@oracle.com>
---
 fs/xfs/xfs_ialloc.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/xfs/xfs_ialloc.h b/fs/xfs/xfs_ialloc.h
index a8f76a5..f4bfd32 100644
--- a/fs/xfs/xfs_ialloc.h
+++ b/fs/xfs/xfs_ialloc.h
@@ -37,6 +37,16 @@ struct xfs_btree_cur;
 #define	XFS_INODE_BIG_CLUSTER_SIZE	8192
 #define	XFS_INODE_CLUSTER_SIZE(mp)	(mp)->m_inode_cluster_size
 
+/* Calculate and return the number of blocks per inode cluster */
+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_blocklog;
+}
+
 /*
  * Make an inode pointer out of the buffer/offset.
  */
-- 
1.8.3.2

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/5] xfs: introduce xfs_ialloc_blks_per_cluster

[Dave Chinner]

On Wed, Dec 11, 2013 at 09:48:01PM +0800, Jeff Liu wrote:
> From: Jie Liu <jeff.liu@oracle.com>
> 
> Introduce a common routine xfs_ialloc_blks_per_cluster() to calculate
> and return the number of blocks per inode cluster.
> 
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> ---
>  fs/xfs/xfs_ialloc.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/xfs/xfs_ialloc.h b/fs/xfs/xfs_ialloc.h
> index a8f76a5..f4bfd32 100644
> --- a/fs/xfs/xfs_ialloc.h
> +++ b/fs/xfs/xfs_ialloc.h
> @@ -37,6 +37,16 @@ struct xfs_btree_cur;
>  #define	XFS_INODE_BIG_CLUSTER_SIZE	8192
>  #define	XFS_INODE_CLUSTER_SIZE(mp)	(mp)->m_inode_cluster_size
>  
> +/* Calculate and return the number of blocks per inode cluster */
> +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_blocklog;
> +}

I'd avoid using the XFS_INODE_CLUSTER_SIZE() macro - it's just a
useless wrapper around mp->m_inode_cluster_size. Hence if you are
cleaning this code up, the first thing I'd do is remove the macro.

/* Calculate and return the number of blocks per inode cluster */
static inline int
xfs_ialloc_blks_per_cluster(
	struct xfs_mount	*mp)
{
	if (mp->m_sb.sb_blocksize >= mp->m_inode_cluster_size)
		return 1;
	return mp->m_inode_cluster_size >> mp->m_sb.sb_blocklog;
}

Is much less shouty, and just as easy to read ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/5] xfs: introduce xfs_ialloc_blks_per_cluster

[Jeff Liu]

On 12/12 2013 07:14 AM, Dave Chinner wrote:
> On Wed, Dec 11, 2013 at 09:48:01PM +0800, Jeff Liu wrote:
>> From: Jie Liu <jeff.liu@oracle.com>
>>
>> Introduce a common routine xfs_ialloc_blks_per_cluster() to calculate
>> and return the number of blocks per inode cluster.
>>
>> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>> ---
>>  fs/xfs/xfs_ialloc.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_ialloc.h b/fs/xfs/xfs_ialloc.h
>> index a8f76a5..f4bfd32 100644
>> --- a/fs/xfs/xfs_ialloc.h
>> +++ b/fs/xfs/xfs_ialloc.h
>> @@ -37,6 +37,16 @@ struct xfs_btree_cur;
>>  #define	XFS_INODE_BIG_CLUSTER_SIZE	8192
>>  #define	XFS_INODE_CLUSTER_SIZE(mp)	(mp)->m_inode_cluster_size
>>  
>> +/* Calculate and return the number of blocks per inode cluster */
>> +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_blocklog;
>> +}
>
> I'd avoid using the XFS_INODE_CLUSTER_SIZE() macro - it's just a
> useless wrapper around mp->m_inode_cluster_size. Hence if you are
> cleaning this code up, the first thing I'd do is remove the macro.
> 
> /* Calculate and return the number of blocks per inode cluster */
> static inline int
> xfs_ialloc_blks_per_cluster(
> 	struct xfs_mount	*mp)
> {
> 	if (mp->m_sb.sb_blocksize >= mp->m_inode_cluster_size)
> 		return 1;
> 	return mp->m_inode_cluster_size >> mp->m_sb.sb_blocklog;
> }
> 
> Is much less shouty, and just as easy to read ;)
Yup, yup! I also considered to just remove this macros as mp->m_inode_cluster_size
is quite understood and actually, we avoid this macros in xfs_inobp_check(). :)
But that means I need to remove it from xlog_recover_buffer_pass2() and fix it's
comments in this patch as well.

Thanks,
-Jeff

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs