Re: [RFC PATCH 2/3] Add pquota fields where gquota is used

[Christoph Hellwig <hch@infradead.org>]

> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -841,8 +841,10 @@ xfs_qm_dqget(
>  		ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  		if (type == XFS_DQ_USER)
>  			ASSERT(ip->i_udquot == NULL);
> -		else
> +		else if (type == XFS_DQ_GROUP)
>  			ASSERT(ip->i_gdquot == NULL);
> +		else
> +			ASSERT(ip->i_pdquot == NULL);

An xfs_inode_dquot(ip, type) macro that gets you the right quota for
a given type would be highly useful.  I'd love to see that as the
first patch in the series, as it would a) clean up a lot of code like
this and b) would also help with my TODO list of overlaying the
user/group dquots with the existing fields in the VFS inode and thus
shrinking the XFS inode.

> @@ -933,8 +935,8 @@ xfs_qm_dqget(
>  				xfs_dqlock(dqp);
>  				goto dqret;
>  			}
> -		} else {
> -			if (!XFS_IS_OQUOTA_ON(mp)) {
> +		} else if (type == XFS_DQ_GROUP) {
> +			if (!XFS_IS_GQUOTA_ON(mp)) {
>  				/* inode stays locked on return */
>  				xfs_qm_dqdestroy(dqp);
>  				return XFS_ERROR(ESRCH);

Together with a

	XFS_IS_TYPE_QUOTA_ON(mp, type)

that should also really greatly simplify this error path in
xfs_qm_dqget.

> +#define XFS_IS_THIS_QUOTA_OFF(d) (! ((XFS_QM_ISUDQ(d) && \
> +				     XFS_IS_UQUOTA_ON((d)->q_mount)) || \
> +				     (XFS_QM_ISGDQ(d) && \
> +				     XFS_IS_GQUOTA_ON((d)->q_mount)) || \
> +				     (XFS_QM_ISPDQ(d) && \
> +				     XFS_IS_PQUOTA_ON((d)->q_mount))))

And the two callers of this could easily be converted to
!XFS_IS_TYPE_QUOTA_ON.

> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 760140d..26f95a6 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -231,6 +231,7 @@ typedef struct xfs_inode {
>  	struct xfs_mount	*i_mount;	/* fs mount struct ptr */
>  	struct xfs_dquot	*i_udquot;	/* user dquot */
>  	struct xfs_dquot	*i_gdquot;	/* group dquot */
> +	struct xfs_dquot	*i_pdquot;	/* project dquot */

I'd really prefer to leave growing the inode until we are fully done
adding it.  So move this part to the back, just add the macro and quota
on check in the first patch, but always redirect it to i_gdquot for now.

> +++ b/fs/xfs/xfs_mount.c
> @@ -628,6 +628,7 @@ xfs_sb_to_disk(
>  	xfs_sb_field_t	f;
>  	int		first;
>  	int		size;
> +	__be16		saved_qflags = from->sb_qflags;
>  
>  	ASSERT(fields);
>  	if (!fields)
> @@ -669,6 +670,7 @@ xfs_sb_to_disk(
>  
>  		fields &= ~(1LL << f);
>  	}
> +	from->sb_qflags = saved_qflags;
>  }

What is this for?  It will need some better documentation.

> -		ASSERT(ip->i_udquot == NULL);
> +		ASSERT_ALWAYS(ip->i_udquot == NULL);

Why the change to ASSERT_ALWAYS here?

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