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
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
