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

[Dave Chinner <david@fromorbit.com>]

On Sun, Sep 05, 2010 at 07:24:35PM +0200, Arkadiusz Miskiewicz wrote:
> On Sunday 05 of September 2010, Christoph Hellwig wrote:
> > Thanks for doing this work Arek, I think it will be useful for some
> > users and defintively is cleaners than what we have now.
> > 
> > > I started doing that by splitting every group+project handling code into
> > > separate group and project one. Added superblock field for project quota.
> > > New feature flag (SEPARATEPQUOTA).
> > 
> > Ok.
> > 
> > > If old filesystem (for SEPARATEQUOTA) is mounted then I'll simply load
> > > sb_gquotino into mp->m_sb.sb_pquotino which I think is enough to handle
> > > old fs (since new kernel operates on separate structures for project
> > > quota).
> > 
> > Do you mean an old filesystem without the SEPARATEQUOTA bit set here?
> 
> Yes. When trying to load existing project quota inode in 
> xfs_qm_init_quotainos() I'm doing:
> 
> +                       if (!xfs_sb_version_hasseparatepquota(&mp->m_sb)) {
> +                               ASSERT(!XFS_IS_GQUOTA_ON(mp));
> +                               mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
> +                               mp->m_sb.sb_gquotino = 0;
> +                       }
> 
> While xfs_mod_sb() now does:
> 
>         /*
>          * Filesystem has no separatepquota turned on,
>          * so we need to store project quota in group
>          * quota inode on disk.
>          */
>         if (!xfs_sb_version_hasseparatepquota(&mp->m_sb)) {
>                 mp->m_sb.sb_gquotino = mp->m_sb.sb_pquotino;
>                 mp->m_sb.sb_pquotino = 0;
>         }
>
>        xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, fields);
>
>  [...]
>         /*
>          * 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.

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.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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