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