Re: [PATCH 1/4] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD

[Jeff Liu <jeff.liu@oracle.com>]

On 07/17/2012 03:32 PM, Christoph Hellwig wrote:

>> +	if ((to->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) &&
>> +	    (to->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
>> +			      XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))) {
>> +		xfs_notice(mp, "Super block has XFS_OQUOTA bits along with "
>> +			"XFS_PQUOTA and/or XFS_GQUOTA bits. Fixing it.\n");
>> +	}
>> +	if (to->sb_qflags & XFS_OQUOTA_ENFD)
>> +		to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
>> +					XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
>> +	if (to->sb_qflags & XFS_OQUOTA_CHKD)
>> +		to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
>> +					XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
>> +	to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> 
> It's suggest factoring this into a xfs_quota_flags_from_disk helper,
> which a) makes this code more clear, and b) can get a comment about what
> is going on.

Exactly!

> 
>> +			/*
>> +			 * The in-core version of sb_qflags do not have
>> +			 * XFS_OQUOTA_* flags, whereas the on-disk version
>> +			 * does.  Save the in-core sb_qflags temporarily,
>> +			 * removing the new XFS_{PG}QUOTA_* flags and re-apply
>> +			 * the old on-disk flags.
>> +			 */
>> +			tmp16 = from->sb_qflags &
>> +					~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
>> +					  XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
>> +			if (from->sb_qflags &
>> +					(XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
>> +				tmp16 |= XFS_OQUOTA_CHKD;
>> +			*(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
> 
> Same here for the other direction.

Ok, I'll take care of it as well.

> 
>> + * Start differentiating group quota and project quota in-core
>> + * using distinct flags, instread of using the combined OQUOTA flags.
>> + *
>> + * Conversion to and from the combined OQUOTA flag (if necessary)
>> + * is done only in xfs_sb{to, from}_disk()
>> + */
>> +#define XFS_GQUOTA_ENFD	0x0080	/* group quota limit enforced */
>> +#define XFS_GQUOTA_CHKD 0x0100	/* quotacheck run on group quotas */
>> +#define XFS_PQUOTA_ENFD	0x0200	/* project quota limits enforced */
>> +#define XFS_PQUOTA_CHKD	0x0800	/* quotacheck run on project quotas.
>> +				 * FIXME: here we cannot using 0x0400
>> +				 * because it is already reserved for
>> +				 * XFS_GQUOTA_ACTIVE.  Otherwise,
>> +				 * XFS_IS_GQUOTA_ON() is also true
>> +				 * although it does not enabled.
>> +				 */
> 
> Seems like all te 0x0?00 values are used for XFS_U?UOTA_ACTIVE flags.
> Why not move it to the 0x?000 range?

I have not took that previously because it will make the code looks a bit in-continuous, consider we have to skip all 0x0?00.
But yes, move PQUOTA_ENFD/PQUOTA_CHKD to 0x?000 range is fine, thanks for the advice!

-Jeff
 .

> 
> Otherwise these changes look good to me.


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