public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Arkadiusz Miskiewicz <arekm@maven.pl>
To: Dave Chinner <david@fromorbit.com>
Cc: Christoph Hellwig <hch@infradead.org>, xfs@oss.sgi.com
Subject: Re: separate project quota from group quota (questions, design issues)
Date: Mon, 6 Sep 2010 08:28:19 +0200	[thread overview]
Message-ID: <201009060828.19924.arekm@maven.pl> (raw)
In-Reply-To: <20100906011213.GY7362@dastard>

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/

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

  reply	other threads:[~2010-09-06  6:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-04  8:00 separate project quota from group quota (questions, design issues) Arkadiusz Miskiewicz
2010-09-04 17:13 ` Michael Monnerie
2010-09-04 23:30 ` Christoph Hellwig
2010-09-05 17:24   ` Arkadiusz Miskiewicz
2010-09-06  1:12     ` Dave Chinner
2010-09-06  6:28       ` Arkadiusz Miskiewicz [this message]
2010-09-06  1:16   ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201009060828.19924.arekm@maven.pl \
    --to=arekm@maven.pl \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox