Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators.

[Christoph Hellwig <hch@infradead.org>]

On Wed, Sep 22, 2010 at 07:42:23PM +0200, Arkadiusz Mi??kiewicz wrote:
>  	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)
> +			xfs_addprojid32bit(tp, ip);

Didn't we agree that we want to enable this feature explicitly via
xfs_admin (or mkfs.xfs)?

> diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h
> index 87c2e9d..c03c752 100644
> --- a/fs/xfs/xfs_fs.h
> +++ b/fs/xfs/xfs_fs.h
> @@ -293,9 +293,10 @@ typedef struct xfs_bstat {
>  	__s32		bs_extsize;	/* extent size			*/
>  	__s32		bs_extents;	/* number of extents		*/
>  	__u32		bs_gen;		/* generation count		*/
> -	__u16		bs_projid;	/* project id			*/
> +	__u16		bs_projid_lo;	/* lower part of project id	*/
>  	__u16		bs_forkoff;	/* inode fork offset in bytes	*/
> -	unsigned char	bs_pad[12];	/* pad space, unused		*/
> +	__u16		bs_projid_hi;	/* higher part of project id	*/
> +	unsigned char	bs_pad[10];	/* pad space, unused		*/

Unlike in the inode we can't just rename the lo field here - that would
break the compilation of existing applications.

>  /*
> + * Project quota id helpers
> + */

Maybe add a little comment here that the split is because the projid
historically was just 16 bits on disk?

> +static inline prid_t
> +xfs_get_projid(xfs_inode_t *ip)

Please always use struct xfs_foo instead of xfs_foo_t for new code.

>  	return 0;
>  }
> +
> +/*
> + * Switches on the PROJID32BIT superblock bit
> + * (implies also FEATURES2).
> + */
> +
> +int

No need for a space after the comment.

> +{
> +	spin_lock(&ip->i_mount->m_sb_lock);
> +	if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb)) {

Wedon't need the lock around the check, it's enough if it's inside the
conditional.

> +		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);

Weird formatting?

Otherwise this looks good to me.

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