xfs
[Top] [All Lists]

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

To: Arkadiusz Miskiewicz <arekm@xxxxxxxx>
Subject: Re: separate project quota from group quota (questions, design issues)
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 6 Sep 2010 11:12:13 +1000
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <201009051924.36443.arekm@xxxxxxxx>
References: <201009041000.55878.arekm@xxxxxxxx> <20100904233053.GA26586@xxxxxxxxxxxxx> <201009051924.36443.arekm@xxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
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@xxxxxxxxxxxxx

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