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

Jeff Liu
2012-07-17T12:45:07Z

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.