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

Ben Myers
2012-02-17T18:39:00Z

Hey Chandra,

On Mon, Feb 13, 2012 at 10:05:57AM -0600, Chandra Seetharaman wrote:
> On Fri, 2012-02-10 at 18:41 -0600, Ben Myers wrote:
>
> <snip>
>
> > > @@ -657,6 +686,7 @@ xfs_sb_to_disk(
> > >
> > > fields &= ~(1LL << f);
> > > }
> > > + from->sb_qflags = saved_qflags;
> >
> > I am disgusted that you should have to save the sb_qflags like this.
> > After reading this 'fields' loop in xfs_sb_to_disk along with all the
> > related crap I am actually feeling a little nauseous.
>
> Let me know if you have any other solutions. I am open for it.

How would you feel about something like this?

@@ -622,6 +636,7 @@ xfs_sb_to_disk(
xfs_sb_field_t f;
int first;
int size;
+ __uint16_t tmp16;

ASSERT(fields);
if (!fields)
@@ -636,6 +651,27 @@ xfs_sb_to_disk(

if (size == 1 || xfs_sb_info[f].type == 1) {
memcpy(to_ptr + first, from_ptr + first, size);
+ } else if (f == XFS_SBS_QFLAGS) {
+ /*
+ * 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. This is a temporary
+ * situation until the on-disk flags are updated.
+ */
+ 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_ENFD;
+ if (from->sb_qflags &
+ (XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
+ tmp16 |= XFS_OQUOTA_CHKD;
+
+ *(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
} else {
switch (size) {
case 2:

This way we needn't worry about any change (or locking) of the incore
superblock sb_qflags.

> > I don't know why in XFS we seem to feel the need to do everything in the
> > most complicated way possible.

I suppose they were trying to cut down on conditionals on fields... maybe
something that could be done better at compile time... someday.

Regards,
Ben