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

Ben Myers bpm at sgi.com
Fri Feb 17 12:39:02 CST 2012


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



More information about the xfs mailing list