Re: [PATCH] Extend project quotas to support 32bit project identificators. [version 3]

[Dave Chinner <david@fromorbit.com>]

On Sat, Aug 28, 2010 at 12:39:58AM +0200, Arkadiusz Miśkiewicz wrote:
> This patch adds support for 32bit project quota identificators.
> 
> On disk format is backward compatible with 16bit projid numbers. projid
> on disk is now keept in two 16bit values - di_projid_lo (which holds the
> same position as old 16bit projid value) and new di_projid_hi (takes
> existing padding) and convertes from/to 32bit value on the fly.
> 
> PROJID32BIT feature2 flag is set automaticly when trying to use 32bit
> quota project identificator.
> 
> Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
.....
> @@ -946,13 +946,30 @@ xfs_ioctl_setattr(
>  		goto error_return;
>  	}
>  
> -	/*
> -	 * Do a quota reservation only if projid is actually going to change.
> -	 */
>  	if (mask & FSX_PROJID) {
> +		/*
> +		 * Switch on the PROJID32BIT superblock bit when needed
> +		 * (implies also FEATURES2)
> +		 */
> +		if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) &&
> +				fa->fsx_projid > (__uint16_t)-1) {
> +			spin_lock(&ip->i_mount->m_sb_lock);
> +			if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb)) {
> +				xfs_sb_version_addprojid32bit(&ip->i_mount->m_sb);
> +				spin_unlock(&ip->i_mount->m_sb_lock);
> +				xfs_mod_sb(tp,
> +					XFS_SB_VERSIONNUM | XFS_SB_FEATURES2);
> +			} else
> +				spin_unlock(&ip->i_mount->m_sb_lock);
> +		}

Can you separate out the superblock feature bit modification into a
helper function?

> @@ -335,6 +336,23 @@ xfs_iflags_test_and_clear(xfs_inode_t *ip, unsigned short flags)
>  }
>  
>  /*
> + * Project quota id helpers
> + */
> +static inline prid_t
> +xfs_get_projid(xfs_inode_t *ip)
> +{
> +	return (prid_t)(ip->i_d.di_projid_hi) << 16 | ip->i_d.di_projid_lo;
> +}

I think the set of parenthesis should be separating to the two
halveѕ of value clearly - the parenthesis around
ip->i_d.di_projid_hi are effectively meaningless. i.e:

	return ((prid_t)ip->i_d.di_projid_hi << 16) | ip->i_d.di_projid_lo;

> +static inline int xfs_sb_version_hasprojid32bit(xfs_sb_t *sbp)
> +{
> +	return xfs_sb_version_hasmorebits(sbp) &&
> +		(sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT);
> +}
> +
> +static inline void xfs_sb_version_addprojid32bit(xfs_sb_t *sbp)
> +{
> +	sbp->sb_versionnum |= XFS_SB_VERSION_MOREBITSBIT;
> +	sbp->sb_features2 |= XFS_SB_VERSION2_PROJID32BIT;
> +}

This also needs to set the bit in the sbp->sb_bad_features2 field.

Otherwise it looks ok. I'll throw it in my QA stack and see how it
goes.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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