xfs
[Top] [All Lists]

Re: separate project quota from group quota (questions, design issues)

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: separate project quota from group quota (questions, design issues)
From: Arkadiusz Miskiewicz <arekm@xxxxxxxx>
Date: Mon, 6 Sep 2010 08:28:19 +0200
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20100906011213.GY7362@dastard>
References: <201009041000.55878.arekm@xxxxxxxx> <201009051924.36443.arekm@xxxxxxxx> <20100906011213.GY7362@dastard>
User-agent: KMail/1.13.5 (Linux/2.6.35-final-dirty; KDE/4.5.1; x86_64; ; )
On Monday 06 of September 2010, Dave Chinner wrote:
> On Sun, Sep 05, 2010 at 07:24:35PM +0200, Arkadiusz Miskiewicz wrote:

> >          * Restore original in-memory project quota inode state.
> >          */
> >         
> >         if (!xfs_sb_version_hasseparatepquota(&mp->m_sb)) {
> >         
> >                 mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
> >                 mp->m_sb.sb_gquotino = 0;
> >         
> >         }
> 
> I don't think that is safe - we can have concurrent access to the
> in-core superblock (mp->m_sb) without locking the superblock, so
> something that races with xfs_mod_sb() looking up project quota
> could die a horrible death here.

I was thinking about working on a copy, too, like this
+       xfs_sb_t        sb_copy;
[...]
+       memcpy(&sb_copy, from, sizeof(sb_copy));
+       from_ptr = (xfs_caddr_t)&sb_copy;
+
+       if (!xfs_sb_version_hasseparatepquota(from) &&
+                       (sb_copy.sb_qflags & XFS_PQUOTA_ACCT)) {
+               sb_copy.sb_gquotino = from->sb_pquotino;
+               sb_copy.sb_pquotino = 0;
+
+       } 

> 
> The only time that you should need to do this juggling is when the
> quota inode changes. That is, when the XFS_SB_GQUOTINO field is
> set. Otherwise the field won't be modified and so we don't need to
> convert the values. That only occurs when quotas are being
> initialised (xfs_qm_qino_alloc()) during mount, so in that case
> there can't be any concurrent operations occurring. Hence swizzling
> the inode fields only when the XFS_SB_GQUOTINO filed is set should
> be safe.

Unfortunately it turns out that I need to convert sb_qflags, too (see below)
and sb_qflags seem to be changes in few different places, so xfs_mod_sb()
looks a good place to do a conversion for this.

sb_qflags come into play. Old XFS_OQUOTA_CHKD and
XFS_PQUOTA_CHKD are no longer used by kernel but existing filesystems
can have these, so I'm going to translate old into new flags currently
in xfs_sb_from_disk() and vice versa in xfs_mod_sb().

@@ -575,6 +588,26 @@ xfs_sb_from_disk(
        to->sb_logsunit = be32_to_cpu(from->sb_logsunit);
        to->sb_features2 = be32_to_cpu(from->sb_features2);
        to->sb_bad_features2 = be32_to_cpu(from->sb_bad_features2);
+       to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
+
+       /*
+        * Convert old quota to separatepquota flags.
+        */
+       if (to->sb_qflags & XFS_OQUOTA_CHKD_NOTUSED) {
+               if (to->sb_qflags & XFS_GQUOTA_ACCT) {
+                       to->sb_qflags &= ~XFS_OQUOTA_CHKD_NOTUSED;
+                       to->sb_qflags |= XFS_GQUOTA_CHKD;
+               } else if (to->sb_qflags & XFS_PQUOTA_ACCT) {
+                       to->sb_qflags &= ~XFS_PQUOTA_CHKD_NOTUSED;
+                       to->sb_qflags |= XFS_PQUOTA_CHKD;
+               }
+       }
+
+       if (!xfs_sb_version_hasseparatepquota(to) &&
+                       (to->sb_qflags & XFS_PQUOTA_ACCT)) {
+               to->sb_pquotino = to->sb_gquotino;
+               to->sb_gquotino = 0;
+       }
 }

New flags are going to be like this:

 #define XFS_UQUOTA_ENFD        0x0002  /* user quota limits enforced */        
                                                                             
 #define XFS_UQUOTA_CHKD        0x0004  /* quotacheck run on usr quotas */      
                                                                             
 #define XFS_PQUOTA_ACCT        0x0008  /* project quota accounting ON */       
                                                                             
-#define XFS_OQUOTA_ENFD        0x0010  /* other (grp/prj) quota limits 
enforced */                                                                     
     
-#define XFS_OQUOTA_CHKD        0x0020  /* quotacheck run on other (grp/prj) 
quotas */                                                                       
+#define XFS_OQUOTA_ENFD_NOTUSED        0x0010  /* other (grp/prj) quota limits 
enforced (NO LONGER USED)*/                                                  
+#define XFS_OQUOTA_CHKD_NOTUSED        0x0020  /* quotacheck run on other 
(grp/prj) quotas (NO LONGER USED)*/                                             
  
 #define XFS_GQUOTA_ACCT        0x0040  /* group quota accounting ON */         
                                                                             
+#define XFS_GQUOTA_ENFD        0x0080  /* group quota limits enforced */       
                                                                             
+#define XFS_GQUOTA_CHKD        0x0100  /* group quota accoungint ON */         
                                                                             
+#define XFS_PQUOTA_CHKD        0x0200  /* quotacheck run on project quotas */  
                                                                             
+#define XFS_PQUOTA_ENFD        0x0400  /* project quota limits enforced */  


> Cheers,
> 
> Dave.


-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

<Prev in Thread] Current Thread [Next in Thread>