* [RFC v6 PATCH 0/5] xfs: Allow pquota and gquota to be used together
@ 2012-07-20 23:02 Chandra Seetharaman
2012-07-20 23:02 ` [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD Chandra Seetharaman
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Chandra Seetharaman @ 2012-07-20 23:02 UTC (permalink / raw)
To: xfs; +Cc: Chandra Seetharaman
Hello All,
This is the version 6 of the changes to allow pquota and gquota to be used
together.
Version 5 can be found at:
http://marc.info/?l=linux-xfs&m=133175714810796&w=2
Changes from version 5 to version 6:
- Use radix tree instead of hash table
Version 4 can be found at
http://oss.sgi.com/archives/xfs/2012-02/msg00528.html
Changes fron version 4 to version 5:
rebase with the current tree one simple change.
Version 3 of the posting can be found at
http://oss.sgi.com/archives/xfs/2012-01/msg00309.html
Changes from version 3 to version 4:
- Remove save_flags with storing the value (in to superblock)
based on field type info
- fix checkpatch.pl warnings and errors
version 2 of the posting can be found at
http://marc.info/?l=linux-xfs&m=131966420607401&w=2
Changes from version 2 to version 3:
- hash table for pquota is added.
- changes to apply cleanly with the latest tree
version 1 of the posting can be found at
http://article.gmane.org/gmane.comp.file-systems.xfs.general/41284
Changes from version 1 to version 2:
- Created a new prep patch to accomodate some generic changes that ease
the later patches.
- Created a new patch to add a new field qs_pquota to fs_quota_stat
with appropriate versioning changes
- Changed the logic to allow XFS_OQUOTA.* flags to be allowed only in
the older versions.
- Changed couple of places where PQUOTA checking was on the else if
construct so as to allow both GQUOTA and PQUOTA in those places.
- Fixed comments in xfs_quota.h to reflect the current changes.
- Changed the name of the macro XFS_SB_VERSION2_SEPER_PQUOTA to
XFS_SB_VERSION2_NO_OQUOTA
- got rid of the macros XFS_MOUNT_QUOTA_SET1 and XFS_MOUNT_QUOTA_SET2
- added a new inline function xfs_inode_dquot(ip, type) to simplify
the error path in xfs_qm_dqget()
- got rid of the macro XFS_IS_THIS_QUOTA_OFF
- added comment to explain why sb_qflags is saved and restored in
xfs_sb_to_disk()
Thanks & Regards,
chandra
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD
2012-07-20 23:02 [RFC v6 PATCH 0/5] xfs: Allow pquota and gquota to be used together Chandra Seetharaman
@ 2012-07-20 23:02 ` Chandra Seetharaman
2012-08-14 22:46 ` Dave Chinner
2012-07-20 23:02 ` [RFC v6 PATCH 2/5] xfs: Add pquota fields where gquota is used Chandra Seetharaman
` (3 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Chandra Seetharaman @ 2012-07-20 23:02 UTC (permalink / raw)
To: xfs; +Cc: Chandra Seetharaman
Remove all incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD. Instead,
start using XFS_GQUOTA_.* XFS_PQUOTA_.* counterparts for GQUOTA nd
PQUOTA respectively.
No changes are made to the on-disk version of the superblock yet. On-disk
copy still uses XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD.
Read and write of the superblock does the conversion from *OQUOTA*
to *[PG]QUOTA*.
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
fs/xfs/xfs_mount.c | 35 +++++++++++++++++++++++++++++++++++
fs/xfs/xfs_qm.c | 9 ++++++---
fs/xfs/xfs_qm_syscalls.c | 28 ++++++++++++++++------------
fs/xfs/xfs_quota.h | 36 +++++++++++++++++++++++++-----------
fs/xfs/xfs_quotaops.c | 6 ++++--
fs/xfs/xfs_super.c | 16 ++++++++--------
fs/xfs/xfs_trans_dquot.c | 4 ++--
7 files changed, 96 insertions(+), 38 deletions(-)
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 9536fd1..22f23fd 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -593,6 +593,20 @@ xfs_sb_from_disk(
to->sb_uquotino = be64_to_cpu(from->sb_uquotino);
to->sb_gquotino = be64_to_cpu(from->sb_gquotino);
to->sb_qflags = be16_to_cpu(from->sb_qflags);
+ if ((to->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) &&
+ (to->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
+ XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))) {
+ xfs_notice(mp, "Super block has XFS_OQUOTA bits along with "
+ "XFS_PQUOTA and/or XFS_GQUOTA bits. Fixing it.\n");
+ }
+ if (to->sb_qflags & XFS_OQUOTA_ENFD)
+ to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
+ XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
+ if (to->sb_qflags & XFS_OQUOTA_CHKD)
+ to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
+ XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
+ to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
+
to->sb_flags = from->sb_flags;
to->sb_shared_vn = from->sb_shared_vn;
to->sb_inoalignmt = be32_to_cpu(from->sb_inoalignmt);
@@ -622,6 +636,7 @@ xfs_sb_to_disk(
xfs_sb_field_t f;
int first;
int size;
+ __uint16_t tmp16;
ASSERT(fields);
if (!fields)
@@ -636,6 +651,26 @@ xfs_sb_to_disk(
if (size == 1 || xfs_sb_info[f].type == 1) {
memcpy(to_ptr + first, from_ptr + first, size);
+ } else if (f == XFS_SBS_QFLAGS) {
+ /*
+ * The in-core version of sb_qflags do not have
+ * XFS_OQUOTA_* flags, whereas the on-disk version
+ * does. Save the in-core sb_qflags temporarily,
+ * removing the new XFS_{PG}QUOTA_* flags and re-apply
+ * the old on-disk flags.
+ */
+ tmp16 = from->sb_qflags &
+ ~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
+ XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
+
+ if (from->sb_qflags &
+ (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
+ tmp16 |= XFS_OQUOTA_ENFD;
+ if (from->sb_qflags &
+ (XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
+ tmp16 |= XFS_OQUOTA_CHKD;
+
+ *(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
} else {
switch (size) {
case 2:
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 2e86fa0..3fb3730 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -297,8 +297,10 @@ xfs_qm_mount_quotas(
*/
if (!XFS_IS_UQUOTA_ON(mp))
mp->m_qflags &= ~XFS_UQUOTA_CHKD;
- if (!(XFS_IS_GQUOTA_ON(mp) || XFS_IS_PQUOTA_ON(mp)))
- mp->m_qflags &= ~XFS_OQUOTA_CHKD;
+ if (!XFS_IS_GQUOTA_ON(mp))
+ mp->m_qflags &= ~XFS_GQUOTA_CHKD;
+ if (!XFS_IS_PQUOTA_ON(mp))
+ mp->m_qflags &= ~XFS_PQUOTA_CHKD;
write_changes:
/*
@@ -1260,7 +1262,8 @@ xfs_qm_quotacheck(
&buffer_list);
if (error)
goto error_return;
- flags |= XFS_OQUOTA_CHKD;
+ flags |= XFS_IS_GQUOTA_ON(mp) ?
+ XFS_GQUOTA_CHKD : XFS_PQUOTA_CHKD;
}
do {
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 858a3b1..76cf0ca 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -116,11 +116,11 @@ xfs_qm_scall_quotaoff(
}
if (flags & XFS_GQUOTA_ACCT) {
dqtype |= XFS_QMOPT_GQUOTA;
- flags |= (XFS_OQUOTA_CHKD | XFS_OQUOTA_ENFD);
+ flags |= (XFS_GQUOTA_CHKD | XFS_GQUOTA_ENFD);
inactivate_flags |= XFS_GQUOTA_ACTIVE;
} else if (flags & XFS_PQUOTA_ACCT) {
dqtype |= XFS_QMOPT_PQUOTA;
- flags |= (XFS_OQUOTA_CHKD | XFS_OQUOTA_ENFD);
+ flags |= (XFS_PQUOTA_CHKD | XFS_PQUOTA_ENFD);
inactivate_flags |= XFS_PQUOTA_ACTIVE;
}
@@ -339,9 +339,11 @@ xfs_qm_scall_quotaon(
||
((flags & XFS_PQUOTA_ACCT) == 0 &&
(mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT) == 0 &&
- (flags & XFS_GQUOTA_ACCT) == 0 &&
+ (flags & XFS_PQUOTA_ENFD))
+ ||
+ ((flags & XFS_GQUOTA_ACCT) == 0 &&
(mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT) == 0 &&
- (flags & XFS_OQUOTA_ENFD))) {
+ (flags & XFS_GQUOTA_ENFD))) {
xfs_debug(mp,
"%s: Can't enforce without acct, flags=%x sbflags=%x\n",
__func__, flags, mp->m_sb.sb_qflags);
@@ -771,8 +773,10 @@ xfs_qm_scall_getquota(
* so return zeroes in that case.
*/
if ((!XFS_IS_UQUOTA_ENFORCED(mp) && dqp->q_core.d_flags == XFS_DQ_USER) ||
- (!XFS_IS_OQUOTA_ENFORCED(mp) &&
- (dqp->q_core.d_flags & (XFS_DQ_PROJ | XFS_DQ_GROUP)))) {
+ (!XFS_IS_PQUOTA_ENFORCED(mp)
+ && dqp->q_core.d_flags == XFS_DQ_PROJ) ||
+ (!XFS_IS_GQUOTA_ENFORCED(mp)
+ && dqp->q_core.d_flags == XFS_DQ_GROUP)) {
dst->d_btimer = 0;
dst->d_itimer = 0;
dst->d_rtbtimer = 0;
@@ -780,8 +784,8 @@ xfs_qm_scall_getquota(
#ifdef DEBUG
if (((XFS_IS_UQUOTA_ENFORCED(mp) && dst->d_flags == FS_USER_QUOTA) ||
- (XFS_IS_OQUOTA_ENFORCED(mp) &&
- (dst->d_flags & (FS_PROJ_QUOTA | FS_GROUP_QUOTA)))) &&
+ (XFS_IS_PQUOTA_ENFORCED(mp) && dst->d_flags == FS_PROJ_QUOTA) ||
+ (XFS_IS_GQUOTA_ENFORCED(mp) && dst->d_flags == FS_GROUP_QUOTA)) &&
dst->d_id != 0) {
if (((int) dst->d_bcount > (int) dst->d_blk_softlimit) &&
(dst->d_blk_softlimit > 0)) {
@@ -833,10 +837,10 @@ xfs_qm_export_flags(
uflags |= FS_QUOTA_GDQ_ACCT;
if (flags & XFS_UQUOTA_ENFD)
uflags |= FS_QUOTA_UDQ_ENFD;
- if (flags & (XFS_OQUOTA_ENFD)) {
- uflags |= (flags & XFS_GQUOTA_ACCT) ?
- FS_QUOTA_GDQ_ENFD : FS_QUOTA_PDQ_ENFD;
- }
+ if (flags & XFS_PQUOTA_ENFD)
+ uflags |= FS_QUOTA_PDQ_ENFD;
+ if (flags & XFS_GQUOTA_ENFD)
+ uflags |= FS_QUOTA_GDQ_ENFD;
return (uflags);
}
diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
index b50ec5b..d7205b0 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -152,19 +152,34 @@ typedef struct xfs_qoff_logformat {
#define XFS_GQUOTA_ACCT 0x0040 /* group quota accounting ON */
/*
+ * Start differentiating group quota and project quota in-core
+ * using distinct flags, instead of using the combined OQUOTA flags.
+ *
+ * Conversion to and from the combined OQUOTA flag (if necessary)
+ * is done only in xfs_sb_{to,from}_disk()
+ */
+#define XFS_GQUOTA_ENFD 0x0080 /* group quota limits enforced */
+#define XFS_GQUOTA_CHKD 0x0100 /* quotacheck run on group quotas */
+#define XFS_PQUOTA_ENFD 0x0200 /* project quota limits enforced */
+#define XFS_PQUOTA_CHKD 0x0400 /* quotacheck run on project quotas */
+
+/*
* Quota Accounting/Enforcement flags
*/
#define XFS_ALL_QUOTA_ACCT \
(XFS_UQUOTA_ACCT | XFS_GQUOTA_ACCT | XFS_PQUOTA_ACCT)
-#define XFS_ALL_QUOTA_ENFD (XFS_UQUOTA_ENFD | XFS_OQUOTA_ENFD)
-#define XFS_ALL_QUOTA_CHKD (XFS_UQUOTA_CHKD | XFS_OQUOTA_CHKD)
+#define XFS_ALL_QUOTA_ENFD \
+ (XFS_UQUOTA_ENFD | XFS_GQUOTA_ENFD | XFS_PQUOTA_ENFD)
+#define XFS_ALL_QUOTA_CHKD \
+ (XFS_UQUOTA_CHKD | XFS_GQUOTA_CHKD | XFS_PQUOTA_CHKD)
#define XFS_IS_QUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_ALL_QUOTA_ACCT)
#define XFS_IS_UQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_UQUOTA_ACCT)
#define XFS_IS_PQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_PQUOTA_ACCT)
#define XFS_IS_GQUOTA_RUNNING(mp) ((mp)->m_qflags & XFS_GQUOTA_ACCT)
#define XFS_IS_UQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_UQUOTA_ENFD)
-#define XFS_IS_OQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_OQUOTA_ENFD)
+#define XFS_IS_PQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_PQUOTA_ENFD)
+#define XFS_IS_GQUOTA_ENFORCED(mp) ((mp)->m_qflags & XFS_GQUOTA_ENFD)
/*
* Incore only flags for quotaoff - these bits get cleared when quota(s)
@@ -259,24 +274,23 @@ typedef struct xfs_qoff_logformat {
((XFS_IS_UQUOTA_ON(mp) && \
(mp->m_sb.sb_qflags & XFS_UQUOTA_CHKD) == 0) || \
(XFS_IS_GQUOTA_ON(mp) && \
- ((mp->m_sb.sb_qflags & XFS_OQUOTA_CHKD) == 0 || \
- (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT))) || \
+ (mp->m_sb.sb_qflags & XFS_GQUOTA_CHKD) == 0) || \
(XFS_IS_PQUOTA_ON(mp) && \
- ((mp->m_sb.sb_qflags & XFS_OQUOTA_CHKD) == 0 || \
- (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT))))
+ (mp->m_sb.sb_qflags & XFS_PQUOTA_CHKD) == 0))
#define XFS_MOUNT_QUOTA_SET1 (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD|\
XFS_UQUOTA_CHKD|XFS_PQUOTA_ACCT|\
- XFS_OQUOTA_ENFD|XFS_OQUOTA_CHKD)
+ XFS_PQUOTA_ENFD|XFS_PQUOTA_CHKD)
#define XFS_MOUNT_QUOTA_SET2 (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD|\
XFS_UQUOTA_CHKD|XFS_GQUOTA_ACCT|\
- XFS_OQUOTA_ENFD|XFS_OQUOTA_CHKD)
+ XFS_GQUOTA_ENFD|XFS_GQUOTA_CHKD)
#define XFS_MOUNT_QUOTA_ALL (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD|\
XFS_UQUOTA_CHKD|XFS_PQUOTA_ACCT|\
- XFS_OQUOTA_ENFD|XFS_OQUOTA_CHKD|\
- XFS_GQUOTA_ACCT)
+ XFS_PQUOTA_ENFD|XFS_PQUOTA_CHKD|\
+ XFS_GQUOTA_ACCT|XFS_GQUOTA_ENFD|\
+ XFS_GQUOTA_CHKD)
/*
diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
index fed504f..6df4faf 100644
--- a/fs/xfs/xfs_quotaops.c
+++ b/fs/xfs/xfs_quotaops.c
@@ -75,8 +75,10 @@ xfs_fs_set_xstate(
flags |= XFS_GQUOTA_ACCT;
if (uflags & FS_QUOTA_UDQ_ENFD)
flags |= XFS_UQUOTA_ENFD;
- if (uflags & (FS_QUOTA_PDQ_ENFD|FS_QUOTA_GDQ_ENFD))
- flags |= XFS_OQUOTA_ENFD;
+ if (uflags & FS_QUOTA_PDQ_ENFD)
+ flags |= XFS_PQUOTA_ENFD;
+ if (uflags & FS_QUOTA_GDQ_ENFD)
+ flags |= XFS_GQUOTA_ENFD;
switch (op) {
case Q_XQUOTAON:
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0d9de41..61ac734 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -338,17 +338,17 @@ xfs_parseargs(
} else if (!strcmp(this_char, MNTOPT_PQUOTA) ||
!strcmp(this_char, MNTOPT_PRJQUOTA)) {
mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
- XFS_OQUOTA_ENFD);
+ XFS_PQUOTA_ENFD);
} else if (!strcmp(this_char, MNTOPT_PQUOTANOENF)) {
mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
- mp->m_qflags &= ~XFS_OQUOTA_ENFD;
+ mp->m_qflags &= ~XFS_PQUOTA_ENFD;
} else if (!strcmp(this_char, MNTOPT_GQUOTA) ||
!strcmp(this_char, MNTOPT_GRPQUOTA)) {
mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
- XFS_OQUOTA_ENFD);
+ XFS_GQUOTA_ENFD);
} else if (!strcmp(this_char, MNTOPT_GQUOTANOENF)) {
mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
- mp->m_qflags &= ~XFS_OQUOTA_ENFD;
+ mp->m_qflags &= ~XFS_GQUOTA_ENFD;
} else if (!strcmp(this_char, MNTOPT_DELAYLOG)) {
xfs_warn(mp,
"delaylog is the default now, option is deprecated.");
@@ -541,12 +541,12 @@ xfs_showargs(
/* Either project or group quotas can be active, not both */
if (mp->m_qflags & XFS_PQUOTA_ACCT) {
- if (mp->m_qflags & XFS_OQUOTA_ENFD)
+ if (mp->m_qflags & XFS_PQUOTA_ENFD)
seq_puts(m, "," MNTOPT_PRJQUOTA);
else
seq_puts(m, "," MNTOPT_PQUOTANOENF);
} else if (mp->m_qflags & XFS_GQUOTA_ACCT) {
- if (mp->m_qflags & XFS_OQUOTA_ENFD)
+ if (mp->m_qflags & XFS_GQUOTA_ENFD)
seq_puts(m, "," MNTOPT_GRPQUOTA);
else
seq_puts(m, "," MNTOPT_GQUOTANOENF);
@@ -1070,8 +1070,8 @@ xfs_fs_statfs(
spin_unlock(&mp->m_sb_lock);
if ((ip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
- ((mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_OQUOTA_ENFD))) ==
- (XFS_PQUOTA_ACCT|XFS_OQUOTA_ENFD))
+ ((mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_PQUOTA_ENFD))) ==
+ (XFS_PQUOTA_ACCT|XFS_PQUOTA_ENFD))
xfs_qm_statvfs(ip, statp);
return 0;
}
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index bcb6054..40460e8 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -638,8 +638,8 @@ xfs_trans_dqresv(
if ((flags & XFS_QMOPT_FORCE_RES) == 0 &&
dqp->q_core.d_id &&
((XFS_IS_UQUOTA_ENFORCED(dqp->q_mount) && XFS_QM_ISUDQ(dqp)) ||
- (XFS_IS_OQUOTA_ENFORCED(dqp->q_mount) &&
- (XFS_QM_ISPDQ(dqp) || XFS_QM_ISGDQ(dqp))))) {
+ (XFS_IS_PQUOTA_ENFORCED(dqp->q_mount) && XFS_QM_ISPDQ(dqp)) ||
+ (XFS_IS_GQUOTA_ENFORCED(dqp->q_mount) && XFS_QM_ISGDQ(dqp)))) {
if (nblks > 0) {
/*
* dquot is locked already. See if we'd go over the
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC v6 PATCH 2/5] xfs: Add pquota fields where gquota is used.
2012-07-20 23:02 [RFC v6 PATCH 0/5] xfs: Allow pquota and gquota to be used together Chandra Seetharaman
2012-07-20 23:02 ` [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD Chandra Seetharaman
@ 2012-07-20 23:02 ` Chandra Seetharaman
2012-08-15 1:15 ` Dave Chinner
2012-07-20 23:02 ` [RFC v6 PATCH 3/5] xfs: Add pquotaino to on-disk super block Chandra Seetharaman
` (2 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Chandra Seetharaman @ 2012-07-20 23:02 UTC (permalink / raw)
To: xfs; +Cc: Chandra Seetharaman
Add project quota changes to all the places where group quota field
is used:
* add separate project quota members into various structures
* split project quota and group quotas so that instead of overriding
the group quota members incore, the new project quota members are
used instead
* get rid of usage of the OQUOTA flag incore, in favor of separate group
and project quota flags.
* add a project dquot argument to various functions.
No externally visible interfaces changed and no superblock changes yet.
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
fs/xfs/xfs_dquot.c | 16 ++-
fs/xfs/xfs_dquot.h | 11 ++-
fs/xfs/xfs_iget.c | 2 +-
fs/xfs/xfs_inode.h | 1 +
fs/xfs/xfs_ioctl.c | 14 ++--
fs/xfs/xfs_iops.c | 4 +-
fs/xfs/xfs_qm.c | 255 ++++++++++++++++++++++++++++++++--------------
fs/xfs/xfs_qm.h | 15 ++--
fs/xfs/xfs_qm_bhv.c | 2 +-
fs/xfs/xfs_qm_syscalls.c | 19 +++-
fs/xfs/xfs_quota.h | 38 ++++---
fs/xfs/xfs_sb.h | 1 +
fs/xfs/xfs_super.c | 5 +-
fs/xfs/xfs_trans_dquot.c | 71 ++++++++++---
fs/xfs/xfs_vnodeops.c | 23 +++--
15 files changed, 326 insertions(+), 151 deletions(-)
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index bf27fcc..42b8b79 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -751,7 +751,7 @@ xfs_qm_dqput_final(
struct xfs_dquot *dqp)
{
struct xfs_quotainfo *qi = dqp->q_mount->m_quotainfo;
- struct xfs_dquot *gdqp;
+ struct xfs_dquot *gdqp, *pdqp;
trace_xfs_dqput_free(dqp);
@@ -765,21 +765,29 @@ xfs_qm_dqput_final(
/*
* If we just added a udquot to the freelist, then we want to release
- * the gdquot reference that it (probably) has. Otherwise it'll keep
- * the gdquot from getting reclaimed.
+ * the gdquot/pdquot reference that it (probably) has. Otherwise it'll
+ * keep the gdquot/pdquot from getting reclaimed.
*/
gdqp = dqp->q_gdquot;
if (gdqp) {
xfs_dqlock(gdqp);
dqp->q_gdquot = NULL;
}
+
+ pdqp = dqp->q_pdquot;
+ if (pdqp) {
+ xfs_dqlock(pdqp);
+ dqp->q_pdquot = NULL;
+ }
xfs_dqunlock(dqp);
/*
- * If we had a group quota hint, release it now.
+ * If we had a group/project quota hint, release it now.
*/
if (gdqp)
xfs_qm_dqput(gdqp);
+ if (pdqp)
+ xfs_qm_dqput(pdqp);
}
/*
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 7d20af2..893cd5e 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -46,6 +46,7 @@ typedef struct xfs_dquot {
xfs_fileoff_t q_fileoffset; /* offset in quotas file */
struct xfs_dquot*q_gdquot; /* group dquot, hint only */
+ struct xfs_dquot *q_pdquot; /* project dquot, hint only */
xfs_disk_dquot_t q_core; /* actual usage & quotas */
xfs_dq_logitem_t q_logitem; /* dquot log item */
xfs_qcnt_t q_res_bcount; /* total regular nblks used+reserved */
@@ -108,8 +109,9 @@ static inline int xfs_this_quota_on(struct xfs_mount *mp, int type)
case XFS_DQ_USER:
return XFS_IS_UQUOTA_ON(mp);
case XFS_DQ_GROUP:
+ return XFS_IS_GQUOTA_ON(mp);
case XFS_DQ_PROJ:
- return XFS_IS_OQUOTA_ON(mp);
+ return XFS_IS_PQUOTA_ON(mp);
default:
return 0;
}
@@ -121,8 +123,9 @@ static inline xfs_dquot_t *xfs_inode_dquot(struct xfs_inode *ip, int type)
case XFS_DQ_USER:
return ip->i_udquot;
case XFS_DQ_GROUP:
- case XFS_DQ_PROJ:
return ip->i_gdquot;
+ case XFS_DQ_PROJ:
+ return ip->i_pdquot;
default:
return NULL;
}
@@ -136,7 +139,9 @@ static inline xfs_dquot_t *xfs_inode_dquot(struct xfs_inode *ip, int type)
#define XFS_DQ_TO_QINF(dqp) ((dqp)->q_mount->m_quotainfo)
#define XFS_DQ_TO_QIP(dqp) (XFS_QM_ISUDQ(dqp) ? \
XFS_DQ_TO_QINF(dqp)->qi_uquotaip : \
- XFS_DQ_TO_QINF(dqp)->qi_gquotaip)
+ (XFS_QM_ISGDQ(dqp) ? \
+ XFS_DQ_TO_QINF(dqp)->qi_gquotaip : \
+ XFS_DQ_TO_QINF(dqp)->qi_pquotaip))
extern int xfs_qm_dqread(struct xfs_mount *, xfs_dqid_t, uint,
uint, struct xfs_dquot **);
diff --git a/fs/xfs/xfs_iget.c b/fs/xfs/xfs_iget.c
index 1bb4365..e97fb18 100644
--- a/fs/xfs/xfs_iget.c
+++ b/fs/xfs/xfs_iget.c
@@ -346,7 +346,7 @@ xfs_iget_cache_miss(
iflags = XFS_INEW;
if (flags & XFS_IGET_DONTCACHE)
iflags |= XFS_IDONTCACHE;
- ip->i_udquot = ip->i_gdquot = NULL;
+ ip->i_udquot = ip->i_gdquot = ip->i_pdquot = NULL;
xfs_iflags_set(ip, iflags);
/* insert the new inode */
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 1efff36..1124620 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -224,6 +224,7 @@ typedef struct xfs_inode {
struct xfs_mount *i_mount; /* fs mount struct ptr */
struct xfs_dquot *i_udquot; /* user dquot */
struct xfs_dquot *i_gdquot; /* group dquot */
+ struct xfs_dquot *i_pdquot; /* project dquot */
/* Inode location stuff */
xfs_ino_t i_ino; /* inode number (agno/agino)*/
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 3a05a41..49da24b 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -907,7 +907,7 @@ xfs_ioctl_setattr(
struct xfs_trans *tp;
unsigned int lock_flags = 0;
struct xfs_dquot *udqp = NULL;
- struct xfs_dquot *gdqp = NULL;
+ struct xfs_dquot *pdqp = NULL;
struct xfs_dquot *olddquot = NULL;
int code;
@@ -936,7 +936,7 @@ xfs_ioctl_setattr(
if (XFS_IS_QUOTA_ON(mp) && (mask & FSX_PROJID)) {
code = xfs_qm_vop_dqalloc(ip, ip->i_d.di_uid,
ip->i_d.di_gid, fa->fsx_projid,
- XFS_QMOPT_PQUOTA, &udqp, &gdqp);
+ XFS_QMOPT_PQUOTA, &udqp, NULL, &pdqp);
if (code)
return code;
}
@@ -973,8 +973,8 @@ xfs_ioctl_setattr(
XFS_IS_PQUOTA_ON(mp) &&
xfs_get_projid(ip) != fa->fsx_projid) {
ASSERT(tp);
- code = xfs_qm_vop_chown_reserve(tp, ip, udqp, gdqp,
- capable(CAP_FOWNER) ?
+ code = xfs_qm_vop_chown_reserve(tp, ip, udqp, NULL,
+ pdqp, capable(CAP_FOWNER) ?
XFS_QMOPT_FORCE_RES : 0);
if (code) /* out of quota */
goto error_return;
@@ -1092,7 +1092,7 @@ xfs_ioctl_setattr(
if (xfs_get_projid(ip) != fa->fsx_projid) {
if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp)) {
olddquot = xfs_qm_vop_chown(tp, ip,
- &ip->i_gdquot, gdqp);
+ &ip->i_pdquot, pdqp);
}
xfs_set_projid(ip, fa->fsx_projid);
@@ -1139,13 +1139,13 @@ xfs_ioctl_setattr(
*/
xfs_qm_dqrele(olddquot);
xfs_qm_dqrele(udqp);
- xfs_qm_dqrele(gdqp);
+ xfs_qm_dqrele(pdqp);
return code;
error_return:
xfs_qm_dqrele(udqp);
- xfs_qm_dqrele(gdqp);
+ xfs_qm_dqrele(pdqp);
xfs_trans_cancel(tp, 0);
if (lock_flags)
xfs_iunlock(ip, lock_flags);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 1a25fd8..e8db154 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -516,7 +516,7 @@ xfs_setattr_nonsize(
ASSERT(udqp == NULL);
ASSERT(gdqp == NULL);
error = xfs_qm_vop_dqalloc(ip, uid, gid, xfs_get_projid(ip),
- qflags, &udqp, &gdqp);
+ qflags, &udqp, &gdqp, NULL);
if (error)
return error;
}
@@ -552,7 +552,7 @@ xfs_setattr_nonsize(
(XFS_IS_GQUOTA_ON(mp) && igid != gid))) {
ASSERT(tp);
error = xfs_qm_vop_chown_reserve(tp, ip, udqp, gdqp,
- capable(CAP_FOWNER) ?
+ NULL, capable(CAP_FOWNER) ?
XFS_QMOPT_FORCE_RES : 0);
if (error) /* out of quota */
goto out_trans_cancel;
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 3fb3730..46c7e4c 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -134,7 +134,7 @@ xfs_qm_dqpurge(
{
struct xfs_mount *mp = dqp->q_mount;
struct xfs_quotainfo *qi = mp->m_quotainfo;
- struct xfs_dquot *gdqp = NULL;
+ struct xfs_dquot *gdqp = NULL, *pdqp = NULL;
xfs_dqlock(dqp);
if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) {
@@ -143,8 +143,8 @@ xfs_qm_dqpurge(
}
/*
- * If this quota has a group hint attached, prepare for releasing it
- * now.
+ * If this quota has a group/project hint attached, prepare for
+ * releasing it now.
*/
gdqp = dqp->q_gdquot;
if (gdqp) {
@@ -152,6 +152,12 @@ xfs_qm_dqpurge(
dqp->q_gdquot = NULL;
}
+ pdqp = dqp->q_pdquot;
+ if (pdqp) {
+ xfs_dqlock(pdqp);
+ dqp->q_pdquot = NULL;
+ }
+
dqp->dq_flags |= XFS_DQ_FREEING;
xfs_dqflock(dqp);
@@ -206,6 +212,8 @@ xfs_qm_dqpurge(
if (gdqp)
xfs_qm_dqput(gdqp);
+ if (pdqp)
+ xfs_qm_dqput(pdqp);
return 0;
}
@@ -362,6 +370,10 @@ xfs_qm_unmount_quotas(
IRELE(mp->m_quotainfo->qi_gquotaip);
mp->m_quotainfo->qi_gquotaip = NULL;
}
+ if (mp->m_quotainfo->qi_pquotaip) {
+ IRELE(mp->m_quotainfo->qi_pquotaip);
+ mp->m_quotainfo->qi_pquotaip = NULL;
+ }
}
}
@@ -408,7 +420,10 @@ xfs_qm_dqattach_one(
* be reclaimed as long as we have a ref from inode and we
* hold the ilock.
*/
- dqp = udqhint->q_gdquot;
+ if (type == XFS_DQ_GROUP)
+ dqp = udqhint->q_gdquot;
+ else
+ dqp = udqhint->q_pdquot;
if (dqp && be32_to_cpu(dqp->q_core.d_id) == id) {
ASSERT(*IO_idqpp == NULL);
@@ -451,28 +466,29 @@ xfs_qm_dqattach_one(
/*
- * Given a udquot and gdquot, attach a ptr to the group dquot in the
+ * Given a udquot and gdquot, attach a ptr to the group/project dquot in the
* udquot as a hint for future lookups.
*/
STATIC void
-xfs_qm_dqattach_grouphint(
- xfs_dquot_t *udq,
- xfs_dquot_t *gdq)
+xfs_qm_dqattach_grouphint(xfs_inode_t *ip, int type)
{
- xfs_dquot_t *tmp;
+ xfs_dquot_t **tmp, *gpdq, *tmp1, *udq = ip->i_udquot;
+ gpdq = (type == XFS_DQ_GROUP) ? ip->i_gdquot : ip->i_pdquot;
xfs_dqlock(udq);
- tmp = udq->q_gdquot;
- if (tmp) {
- if (tmp == gdq)
+ tmp = (type == XFS_DQ_GROUP) ? &udq->q_gdquot : &udq->q_pdquot;
+
+ if (*tmp) {
+ if (*tmp == gpdq)
goto done;
- udq->q_gdquot = NULL;
- xfs_qm_dqrele(tmp);
+ tmp1 = *tmp;
+ *tmp = NULL;
+ xfs_qm_dqrele(tmp1);
}
- udq->q_gdquot = xfs_qm_dqhold(gdq);
+ *tmp = xfs_qm_dqhold(gpdq);
done:
xfs_dqunlock(udq);
}
@@ -526,12 +542,8 @@ xfs_qm_dqattach_locked(
}
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
- if (XFS_IS_OQUOTA_ON(mp)) {
- error = XFS_IS_GQUOTA_ON(mp) ?
- xfs_qm_dqattach_one(ip, ip->i_d.di_gid, XFS_DQ_GROUP,
- flags & XFS_QMOPT_DQALLOC,
- ip->i_udquot, &ip->i_gdquot) :
- xfs_qm_dqattach_one(ip, xfs_get_projid(ip), XFS_DQ_PROJ,
+ if (XFS_IS_GQUOTA_ON(mp)) {
+ error = xfs_qm_dqattach_one(ip, ip->i_d.di_gid, XFS_DQ_GROUP,
flags & XFS_QMOPT_DQALLOC,
ip->i_udquot, &ip->i_gdquot);
/*
@@ -543,14 +555,28 @@ xfs_qm_dqattach_locked(
nquotas++;
}
+ ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ if (XFS_IS_PQUOTA_ON(mp)) {
+ error = xfs_qm_dqattach_one(ip, xfs_get_projid(ip), XFS_DQ_PROJ,
+ flags & XFS_QMOPT_DQALLOC,
+ ip->i_udquot, &ip->i_pdquot);
+ /*
+ * Don't worry about the udquot that we may have
+ * attached above. It'll get detached, if not already.
+ */
+ if (error)
+ goto done;
+ nquotas++;
+ }
+
/*
- * Attach this group quota to the user quota as a hint.
+ * Attach this group/project quota to the user quota as a hint.
* This WON'T, in general, result in a thrash.
*/
- if (nquotas == 2) {
+ if (nquotas > 1 && ip->i_udquot) {
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
- ASSERT(ip->i_udquot);
- ASSERT(ip->i_gdquot);
+ ASSERT(ip->i_gdquot || !XFS_IS_GQUOTA_ON(mp));
+ ASSERT(ip->i_pdquot || !XFS_IS_PQUOTA_ON(mp));
/*
* We do not have i_udquot locked at this point, but this check
@@ -558,8 +584,13 @@ xfs_qm_dqattach_locked(
* 100% all the time. It is just a hint, and this will
* succeed in general.
*/
- if (ip->i_udquot->q_gdquot != ip->i_gdquot)
- xfs_qm_dqattach_grouphint(ip->i_udquot, ip->i_gdquot);
+ if (XFS_IS_GQUOTA_ON(mp) &&
+ ip->i_udquot->q_gdquot != ip->i_gdquot)
+ xfs_qm_dqattach_grouphint(ip, XFS_DQ_GROUP);
+
+ if (XFS_IS_PQUOTA_ON(mp) &&
+ ip->i_udquot->q_pdquot != ip->i_pdquot)
+ xfs_qm_dqattach_grouphint(ip, XFS_DQ_PROJ);
}
done:
@@ -567,8 +598,10 @@ xfs_qm_dqattach_locked(
if (!error) {
if (XFS_IS_UQUOTA_ON(mp))
ASSERT(ip->i_udquot);
- if (XFS_IS_OQUOTA_ON(mp))
+ if (XFS_IS_GQUOTA_ON(mp))
ASSERT(ip->i_gdquot);
+ if (XFS_IS_PQUOTA_ON(mp))
+ ASSERT(ip->i_pdquot);
}
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
#endif
@@ -601,7 +634,7 @@ void
xfs_qm_dqdetach(
xfs_inode_t *ip)
{
- if (!(ip->i_udquot || ip->i_gdquot))
+ if (!(ip->i_udquot || ip->i_gdquot || ip->i_pdquot))
return;
trace_xfs_dquot_dqdetach(ip);
@@ -616,6 +649,10 @@ xfs_qm_dqdetach(
xfs_qm_dqrele(ip->i_gdquot);
ip->i_gdquot = NULL;
}
+ if (ip->i_pdquot) {
+ xfs_qm_dqrele(ip->i_pdquot);
+ ip->i_pdquot = NULL;
+ }
}
/*
@@ -646,6 +683,7 @@ xfs_qm_init_quotainfo(
INIT_RADIX_TREE(&qinf->qi_uquota_tree, GFP_NOFS);
INIT_RADIX_TREE(&qinf->qi_gquota_tree, GFP_NOFS);
+ INIT_RADIX_TREE(&qinf->qi_pquota_tree, GFP_NOFS);
mutex_init(&qinf->qi_tree_lock);
INIT_LIST_HEAD(&qinf->qi_lru_list);
@@ -748,6 +786,10 @@ xfs_qm_destroy_quotainfo(
IRELE(qi->qi_gquotaip);
qi->qi_gquotaip = NULL;
}
+ if (qi->qi_pquotaip) {
+ IRELE(qi->qi_pquotaip);
+ qi->qi_pquotaip = NULL;
+ }
mutex_destroy(&qi->qi_quotaofflock);
kmem_free(qi);
mp->m_quotainfo = NULL;
@@ -1227,7 +1269,7 @@ xfs_qm_quotacheck(
int done, count, error, error2;
xfs_ino_t lastino;
size_t structsz;
- xfs_inode_t *uip, *gip;
+ xfs_inode_t *uip, *gip, *pip;
uint flags;
LIST_HEAD (buffer_list);
@@ -1236,7 +1278,8 @@ xfs_qm_quotacheck(
lastino = 0;
flags = 0;
- ASSERT(mp->m_quotainfo->qi_uquotaip || mp->m_quotainfo->qi_gquotaip);
+ ASSERT(mp->m_quotainfo->qi_uquotaip || mp->m_quotainfo->qi_gquotaip
+ || mp->m_quotainfo->qi_pquotaip);
ASSERT(XFS_IS_QUOTA_RUNNING(mp));
xfs_notice(mp, "Quotacheck needed: Please wait.");
@@ -1257,13 +1300,20 @@ xfs_qm_quotacheck(
gip = mp->m_quotainfo->qi_gquotaip;
if (gip) {
- error = xfs_qm_dqiterate(mp, gip, XFS_IS_GQUOTA_ON(mp) ?
- XFS_QMOPT_GQUOTA : XFS_QMOPT_PQUOTA,
+ error = xfs_qm_dqiterate(mp, gip, XFS_QMOPT_GQUOTA,
&buffer_list);
if (error)
goto error_return;
- flags |= XFS_IS_GQUOTA_ON(mp) ?
- XFS_GQUOTA_CHKD : XFS_PQUOTA_CHKD;
+ flags |= XFS_GQUOTA_CHKD;
+ }
+
+ pip = mp->m_quotainfo->qi_pquotaip;
+ if (pip) {
+ error = xfs_qm_dqiterate(mp, pip, XFS_QMOPT_PQUOTA,
+ &buffer_list);
+ if (error)
+ goto error_return;
+ flags |= XFS_PQUOTA_CHKD;
}
do {
@@ -1358,13 +1408,13 @@ STATIC int
xfs_qm_init_quotainos(
xfs_mount_t *mp)
{
- xfs_inode_t *uip, *gip;
+ xfs_inode_t *uip, *gip, *pip;
int error;
__int64_t sbflags;
uint flags;
ASSERT(mp->m_quotainfo);
- uip = gip = NULL;
+ uip = gip = pip = NULL;
sbflags = 0;
flags = 0;
@@ -1379,7 +1429,7 @@ xfs_qm_init_quotainos(
0, 0, &uip)))
return XFS_ERROR(error);
}
- if (XFS_IS_OQUOTA_ON(mp) &&
+ if (XFS_IS_GQUOTA_ON(mp) &&
mp->m_sb.sb_gquotino != NULLFSINO) {
ASSERT(mp->m_sb.sb_gquotino > 0);
if ((error = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino,
@@ -1389,6 +1439,19 @@ xfs_qm_init_quotainos(
return XFS_ERROR(error);
}
}
+ if (XFS_IS_PQUOTA_ON(mp) &&
+ mp->m_sb.sb_pquotino != NULLFSINO) {
+ ASSERT(mp->m_sb.sb_pquotino > 0);
+ error = xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
+ 0, 0, &pip);
+ if (error) {
+ if (uip)
+ IRELE(uip);
+ if (gip)
+ IRELE(gip);
+ return XFS_ERROR(error);
+ }
+ }
} else {
flags |= XFS_QMOPT_SBVERSION;
sbflags |= (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
@@ -1396,7 +1459,7 @@ xfs_qm_init_quotainos(
}
/*
- * Create the two inodes, if they don't exist already. The changes
+ * Create the three inodes, if they don't exist already. The changes
* made above will get added to a transaction and logged in one of
* the qino_alloc calls below. If the device is readonly,
* temporarily switch to read-write to do this.
@@ -1409,14 +1472,26 @@ xfs_qm_init_quotainos(
flags &= ~XFS_QMOPT_SBVERSION;
}
- if (XFS_IS_OQUOTA_ON(mp) && gip == NULL) {
- flags |= (XFS_IS_GQUOTA_ON(mp) ?
- XFS_QMOPT_GQUOTA : XFS_QMOPT_PQUOTA);
+ if (XFS_IS_GQUOTA_ON(mp) && gip == NULL) {
error = xfs_qm_qino_alloc(mp, &gip,
- sbflags | XFS_SB_GQUOTINO, flags);
+ sbflags | XFS_SB_GQUOTINO,
+ flags | XFS_QMOPT_GQUOTA);
+ if (error) {
+ if (uip)
+ IRELE(uip);
+
+ return XFS_ERROR(error);
+ }
+ }
+ if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
+ error = xfs_qm_qino_alloc(mp, &pip,
+ sbflags | XFS_SB_GQUOTINO,
+ flags | XFS_QMOPT_PQUOTA);
if (error) {
if (uip)
IRELE(uip);
+ if (gip)
+ IRELE(gip);
return XFS_ERROR(error);
}
@@ -1424,6 +1499,7 @@ xfs_qm_init_quotainos(
mp->m_quotainfo->qi_uquotaip = uip;
mp->m_quotainfo->qi_gquotaip = gip;
+ mp->m_quotainfo->qi_pquotaip = pip;
return 0;
}
@@ -1621,10 +1697,11 @@ xfs_qm_vop_dqalloc(
prid_t prid,
uint flags,
struct xfs_dquot **O_udqpp,
- struct xfs_dquot **O_gdqpp)
+ struct xfs_dquot **O_gdqpp,
+ struct xfs_dquot **O_pdqpp)
{
struct xfs_mount *mp = ip->i_mount;
- struct xfs_dquot *uq, *gq;
+ struct xfs_dquot *uq, *gq, *pq;
int error;
uint lockflags;
@@ -1649,7 +1726,7 @@ xfs_qm_vop_dqalloc(
}
}
- uq = gq = NULL;
+ uq = gq = pq = NULL;
if ((flags & XFS_QMOPT_UQUOTA) && XFS_IS_UQUOTA_ON(mp)) {
if (ip->i_d.di_uid != uid) {
/*
@@ -1705,25 +1782,28 @@ xfs_qm_vop_dqalloc(
ASSERT(ip->i_gdquot);
gq = xfs_qm_dqhold(ip->i_gdquot);
}
- } else if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
+ }
+ if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
if (xfs_get_projid(ip) != prid) {
xfs_iunlock(ip, lockflags);
if ((error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)prid,
XFS_DQ_PROJ,
XFS_QMOPT_DQALLOC |
XFS_QMOPT_DOWARN,
- &gq))) {
+ &pq))) {
if (uq)
xfs_qm_dqrele(uq);
+ if (gq)
+ xfs_qm_dqrele(gq);
ASSERT(error != ENOENT);
return (error);
}
- xfs_dqunlock(gq);
+ xfs_dqunlock(pq);
lockflags = XFS_ILOCK_SHARED;
xfs_ilock(ip, lockflags);
} else {
- ASSERT(ip->i_gdquot);
- gq = xfs_qm_dqhold(ip->i_gdquot);
+ ASSERT(ip->i_pdquot);
+ pq = xfs_qm_dqhold(ip->i_pdquot);
}
}
if (uq)
@@ -1738,6 +1818,10 @@ xfs_qm_vop_dqalloc(
*O_gdqpp = gq;
else if (gq)
xfs_qm_dqrele(gq);
+ if (O_pdqpp)
+ *O_pdqpp = pq;
+ else if (pq)
+ xfs_qm_dqrele(pq);
return 0;
}
@@ -1790,11 +1874,13 @@ xfs_qm_vop_chown_reserve(
xfs_inode_t *ip,
xfs_dquot_t *udqp,
xfs_dquot_t *gdqp,
+ xfs_dquot_t *pdqp,
uint flags)
{
xfs_mount_t *mp = ip->i_mount;
uint delblks, blkflags, prjflags = 0;
- xfs_dquot_t *unresudq, *unresgdq, *delblksudq, *delblksgdq;
+ xfs_dquot_t *unresudq, *unresgdq, *unrespdq;
+ xfs_dquot_t *delblksudq, *delblksgdq, *delblkspdq;
int error;
@@ -1802,7 +1888,8 @@ xfs_qm_vop_chown_reserve(
ASSERT(XFS_IS_QUOTA_RUNNING(mp));
delblks = ip->i_delayed_blks;
- delblksudq = delblksgdq = unresudq = unresgdq = NULL;
+ delblksudq = delblksgdq = delblkspdq = NULL;
+ unresudq = unresgdq = unrespdq = NULL;
blkflags = XFS_IS_REALTIME_INODE(ip) ?
XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS;
@@ -1819,25 +1906,28 @@ xfs_qm_vop_chown_reserve(
unresudq = ip->i_udquot;
}
}
- if (XFS_IS_OQUOTA_ON(ip->i_mount) && gdqp) {
- if (XFS_IS_PQUOTA_ON(ip->i_mount) &&
- xfs_get_projid(ip) != be32_to_cpu(gdqp->q_core.d_id))
- prjflags = XFS_QMOPT_ENOSPC;
-
- if (prjflags ||
- (XFS_IS_GQUOTA_ON(ip->i_mount) &&
- ip->i_d.di_gid != be32_to_cpu(gdqp->q_core.d_id))) {
- delblksgdq = gdqp;
- if (delblks) {
- ASSERT(ip->i_gdquot);
- unresgdq = ip->i_gdquot;
- }
+ if (XFS_IS_GQUOTA_ON(ip->i_mount) && gdqp &&
+ ip->i_d.di_gid != be32_to_cpu(gdqp->q_core.d_id)) {
+ delblksgdq = gdqp;
+ if (delblks) {
+ ASSERT(ip->i_gdquot);
+ unresgdq = ip->i_gdquot;
+ }
+ }
+
+ if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
+ xfs_get_projid(ip) != be32_to_cpu(pdqp->q_core.d_id)) {
+ prjflags = XFS_QMOPT_ENOSPC;
+ delblkspdq = pdqp;
+ if (delblks) {
+ ASSERT(ip->i_pdquot);
+ unrespdq = ip->i_pdquot;
}
}
if ((error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
- delblksudq, delblksgdq, ip->i_d.di_nblocks, 1,
- flags | blkflags | prjflags)))
+ delblksudq, delblksgdq, delblkspdq, ip->i_d.di_nblocks,
+ 1, flags | blkflags | prjflags)))
return (error);
/*
@@ -1850,15 +1940,16 @@ xfs_qm_vop_chown_reserve(
/*
* Do the reservations first. Unreservation can't fail.
*/
- ASSERT(delblksudq || delblksgdq);
- ASSERT(unresudq || unresgdq);
+ ASSERT(delblksudq || delblksgdq || delblkspdq);
+ ASSERT(unresudq || unresgdq || unrespdq);
if ((error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
- delblksudq, delblksgdq, (xfs_qcnt_t)delblks, 0,
+ delblksudq, delblksgdq, delblkspdq,
+ (xfs_qcnt_t)delblks, 0,
flags | blkflags | prjflags)))
return (error);
xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
- unresudq, unresgdq, -((xfs_qcnt_t)delblks), 0,
- blkflags);
+ unresudq, unresgdq, unrespdq,
+ -((xfs_qcnt_t)delblks), 0, blkflags);
}
return (0);
@@ -1897,7 +1988,8 @@ xfs_qm_vop_create_dqattach(
struct xfs_trans *tp,
struct xfs_inode *ip,
struct xfs_dquot *udqp,
- struct xfs_dquot *gdqp)
+ struct xfs_dquot *gdqp,
+ struct xfs_dquot *pdqp)
{
struct xfs_mount *mp = tp->t_mountp;
@@ -1917,13 +2009,18 @@ xfs_qm_vop_create_dqattach(
}
if (gdqp) {
ASSERT(ip->i_gdquot == NULL);
- ASSERT(XFS_IS_OQUOTA_ON(mp));
- ASSERT((XFS_IS_GQUOTA_ON(mp) ?
- ip->i_d.di_gid : xfs_get_projid(ip)) ==
- be32_to_cpu(gdqp->q_core.d_id));
-
+ ASSERT(XFS_ISGOQUOTA_ON(mp));
+ ASSERT(ip->i_d.di_gid == be32_to_cpu(gdqp->q_core.d_id));
ip->i_gdquot = xfs_qm_dqhold(gdqp);
xfs_trans_mod_dquot(tp, gdqp, XFS_TRANS_DQ_ICOUNT, 1);
}
+ if (pdqp) {
+ ASSERT(ip->i_pdquot == NULL);
+ ASSERT(XFS_IS_PQUOTA_ON(mp));
+ ASSERT(xfs_get_projid(ip) == be32_to_cpu(pdqp->q_core.d_id));
+
+ ip->i_pdquot = xfs_qm_dqhold(pdqp);
+ xfs_trans_mod_dquot(tp, pdqp, XFS_TRANS_DQ_ICOUNT, 1);
+ }
}
diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
index 44b858b..256ff71 100644
--- a/fs/xfs/xfs_qm.h
+++ b/fs/xfs/xfs_qm.h
@@ -44,9 +44,11 @@ extern struct kmem_zone *xfs_qm_dqtrxzone;
typedef struct xfs_quotainfo {
struct radix_tree_root qi_uquota_tree;
struct radix_tree_root qi_gquota_tree;
+ struct radix_tree_root qi_pquota_tree;
struct mutex qi_tree_lock;
xfs_inode_t *qi_uquotaip; /* user quota inode */
xfs_inode_t *qi_gquotaip; /* group quota inode */
+ xfs_inode_t *qi_pquotaip; /* project quota inode */
struct list_head qi_lru_list;
struct mutex qi_lru_lock;
int qi_lru_count;
@@ -70,26 +72,25 @@ typedef struct xfs_quotainfo {
} xfs_quotainfo_t;
#define XFS_DQUOT_TREE(qi, type) \
- ((type & XFS_DQ_USER) ? \
- &((qi)->qi_uquota_tree) : \
- &((qi)->qi_gquota_tree))
+ ((type & XFS_DQ_USER) ? &((qi)->qi_uquota_tree) : \
+ ((type & XFS_DQ_GROUP) ? &((qi)->qi_gquota_tree) : \
+ &((qi)->qi_pquota_tree)))
extern void xfs_trans_mod_dquot(xfs_trans_t *, xfs_dquot_t *, uint, long);
-extern int xfs_trans_reserve_quota_bydquots(xfs_trans_t *, xfs_mount_t *,
- xfs_dquot_t *, xfs_dquot_t *, long, long, uint);
extern void xfs_trans_dqjoin(xfs_trans_t *, xfs_dquot_t *);
extern void xfs_trans_log_dquot(xfs_trans_t *, xfs_dquot_t *);
/*
- * We keep the usr and grp dquots separately so that locking will be easier
- * to do at commit time. All transactions that we know of at this point
+ * We keep the usr, grp, and prj dquots separately so that locking will be
+ * easier to do at commit time. All transactions that we know of at this point
* affect no more than two dquots of one type. Hence, the TRANS_MAXDQS value.
*/
#define XFS_QM_TRANS_MAXDQS 2
typedef struct xfs_dquot_acct {
xfs_dqtrx_t dqa_usrdquots[XFS_QM_TRANS_MAXDQS];
xfs_dqtrx_t dqa_grpdquots[XFS_QM_TRANS_MAXDQS];
+ xfs_dqtrx_t dqa_prjdquots[XFS_QM_TRANS_MAXDQS];
} xfs_dquot_acct_t;
/*
diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
index 6b39115..eb45424 100644
--- a/fs/xfs/xfs_qm_bhv.c
+++ b/fs/xfs/xfs_qm_bhv.c
@@ -115,7 +115,7 @@ xfs_qm_newmount(
(pquotaondisk && !XFS_IS_PQUOTA_ON(mp)) ||
(!pquotaondisk && XFS_IS_PQUOTA_ON(mp)) ||
(gquotaondisk && !XFS_IS_GQUOTA_ON(mp)) ||
- (!gquotaondisk && XFS_IS_OQUOTA_ON(mp))) &&
+ (!gquotaondisk && XFS_IS_GQUOTA_ON(mp))) &&
xfs_dev_is_read_only(mp, "changing quota state")) {
xfs_warn(mp, "please mount with%s%s%s%s.",
(!quotaondisk ? "out quota" : ""),
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 76cf0ca..509bbf7 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -118,7 +118,8 @@ xfs_qm_scall_quotaoff(
dqtype |= XFS_QMOPT_GQUOTA;
flags |= (XFS_GQUOTA_CHKD | XFS_GQUOTA_ENFD);
inactivate_flags |= XFS_GQUOTA_ACTIVE;
- } else if (flags & XFS_PQUOTA_ACCT) {
+ }
+ if (flags & XFS_PQUOTA_ACCT) {
dqtype |= XFS_QMOPT_PQUOTA;
flags |= (XFS_PQUOTA_CHKD | XFS_PQUOTA_ENFD);
inactivate_flags |= XFS_PQUOTA_ACTIVE;
@@ -213,10 +214,14 @@ xfs_qm_scall_quotaoff(
IRELE(q->qi_uquotaip);
q->qi_uquotaip = NULL;
}
- if ((dqtype & (XFS_QMOPT_GQUOTA|XFS_QMOPT_PQUOTA)) && q->qi_gquotaip) {
+ if ((dqtype & XFS_QMOPT_GQUOTA) && q->qi_gquotaip) {
IRELE(q->qi_gquotaip);
q->qi_gquotaip = NULL;
}
+ if ((dqtype & XFS_QMOPT_PQUOTA) && q->qi_pquotaip) {
+ IRELE(q->qi_pquotaip);
+ q->qi_pquotaip = NULL;
+ }
out_unlock:
mutex_unlock(&q->qi_quotaofflock);
@@ -853,9 +858,11 @@ xfs_dqrele_inode(
{
/* skip quota inodes */
if (ip == ip->i_mount->m_quotainfo->qi_uquotaip ||
- ip == ip->i_mount->m_quotainfo->qi_gquotaip) {
+ ip == ip->i_mount->m_quotainfo->qi_gquotaip ||
+ ip == ip->i_mount->m_quotainfo->qi_pquotaip) {
ASSERT(ip->i_udquot == NULL);
ASSERT(ip->i_gdquot == NULL);
+ ASSERT(ip->i_pdquot == NULL);
return 0;
}
@@ -864,10 +871,14 @@ xfs_dqrele_inode(
xfs_qm_dqrele(ip->i_udquot);
ip->i_udquot = NULL;
}
- if (flags & (XFS_PQUOTA_ACCT|XFS_GQUOTA_ACCT) && ip->i_gdquot) {
+ if ((flags & XFS_GQUOTA_ACCT) && ip->i_gdquot) {
xfs_qm_dqrele(ip->i_gdquot);
ip->i_gdquot = NULL;
}
+ if ((flags & XFS_PQUOTA_ACCT) && ip->i_pdquot) {
+ xfs_qm_dqrele(ip->i_pdquot);
+ ip->i_pdquot = NULL;
+ }
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return 0;
}
diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
index d7205b0..1c61c9b 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -186,9 +186,9 @@ typedef struct xfs_qoff_logformat {
* are in the process of getting turned off. These flags are in m_qflags but
* never in sb_qflags.
*/
-#define XFS_UQUOTA_ACTIVE 0x0100 /* uquotas are being turned off */
-#define XFS_PQUOTA_ACTIVE 0x0200 /* pquotas are being turned off */
-#define XFS_GQUOTA_ACTIVE 0x0400 /* gquotas are being turned off */
+#define XFS_UQUOTA_ACTIVE 0x1000 /* uquotas are being turned off */
+#define XFS_PQUOTA_ACTIVE 0x2000 /* pquotas are being turned off */
+#define XFS_GQUOTA_ACTIVE 0x4000 /* gquotas are being turned off */
#define XFS_ALL_QUOTA_ACTIVE \
(XFS_UQUOTA_ACTIVE | XFS_PQUOTA_ACTIVE | XFS_GQUOTA_ACTIVE)
@@ -199,8 +199,6 @@ typedef struct xfs_qoff_logformat {
#define XFS_IS_QUOTA_ON(mp) ((mp)->m_qflags & (XFS_UQUOTA_ACTIVE | \
XFS_GQUOTA_ACTIVE | \
XFS_PQUOTA_ACTIVE))
-#define XFS_IS_OQUOTA_ON(mp) ((mp)->m_qflags & (XFS_GQUOTA_ACTIVE | \
- XFS_PQUOTA_ACTIVE))
#define XFS_IS_UQUOTA_ON(mp) ((mp)->m_qflags & XFS_UQUOTA_ACTIVE)
#define XFS_IS_GQUOTA_ON(mp) ((mp)->m_qflags & XFS_GQUOTA_ACTIVE)
#define XFS_IS_PQUOTA_ON(mp) ((mp)->m_qflags & XFS_PQUOTA_ACTIVE)
@@ -267,8 +265,10 @@ typedef struct xfs_qoff_logformat {
*/
#define XFS_NOT_DQATTACHED(mp, ip) ((XFS_IS_UQUOTA_ON(mp) &&\
(ip)->i_udquot == NULL) || \
- (XFS_IS_OQUOTA_ON(mp) && \
- (ip)->i_gdquot == NULL))
+ (XFS_IS_GQUOTA_ON(mp) && \
+ (ip)->i_gdquot == NULL) || \
+ (XFS_IS_PQUOTA_ON(mp) && \
+ (ip)->i_pdquot == NULL))
#define XFS_QM_NEED_QUOTACHECK(mp) \
((XFS_IS_UQUOTA_ON(mp) && \
@@ -323,17 +323,18 @@ extern int xfs_trans_reserve_quota_nblks(struct xfs_trans *,
struct xfs_inode *, long, long, uint);
extern int xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
struct xfs_mount *, struct xfs_dquot *,
- struct xfs_dquot *, long, long, uint);
+ struct xfs_dquot *, struct xfs_dquot *, long, long, uint);
extern int xfs_qm_vop_dqalloc(struct xfs_inode *, uid_t, gid_t, prid_t, uint,
- struct xfs_dquot **, struct xfs_dquot **);
+ struct xfs_dquot **, struct xfs_dquot **, struct xfs_dquot **);
extern void xfs_qm_vop_create_dqattach(struct xfs_trans *, struct xfs_inode *,
- struct xfs_dquot *, struct xfs_dquot *);
+ struct xfs_dquot *, struct xfs_dquot *, struct xfs_dquot *);
extern int xfs_qm_vop_rename_dqattach(struct xfs_inode **);
extern struct xfs_dquot *xfs_qm_vop_chown(struct xfs_trans *,
struct xfs_inode *, struct xfs_dquot **, struct xfs_dquot *);
extern int xfs_qm_vop_chown_reserve(struct xfs_trans *, struct xfs_inode *,
- struct xfs_dquot *, struct xfs_dquot *, uint);
+ struct xfs_dquot *, struct xfs_dquot *,
+ struct xfs_dquot *, uint);
extern int xfs_qm_dqattach(struct xfs_inode *, uint);
extern int xfs_qm_dqattach_locked(struct xfs_inode *, uint);
extern void xfs_qm_dqdetach(struct xfs_inode *);
@@ -347,10 +348,12 @@ extern void xfs_qm_unmount_quotas(struct xfs_mount *);
#else
static inline int
xfs_qm_vop_dqalloc(struct xfs_inode *ip, uid_t uid, gid_t gid, prid_t prid,
- uint flags, struct xfs_dquot **udqp, struct xfs_dquot **gdqp)
+ uint flags, struct xfs_dquot **udqp, struct xfs_dquot **gdqp,
+ xfs_dquot **pdqp)
{
*udqp = NULL;
*gdqp = NULL;
+ *pdqp = NULL;
return 0;
}
#define xfs_trans_dup_dqinfo(tp, tp2)
@@ -365,14 +368,15 @@ static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *tp,
}
static inline int xfs_trans_reserve_quota_bydquots(struct xfs_trans *tp,
struct xfs_mount *mp, struct xfs_dquot *udqp,
- struct xfs_dquot *gdqp, long nblks, long nions, uint flags)
+ struct xfs_dquot *gdqp, struct xfs_dquot *pdqp,
+ long nblks, long nions, uint flags)
{
return 0;
}
-#define xfs_qm_vop_create_dqattach(tp, ip, u, g)
+#define xfs_qm_vop_create_dqattach(tp, ip, u, g, p)
#define xfs_qm_vop_rename_dqattach(it) (0)
#define xfs_qm_vop_chown(tp, ip, old, new) (NULL)
-#define xfs_qm_vop_chown_reserve(tp, ip, u, g, fl) (0)
+#define xfs_qm_vop_chown_reserve(tp, ip, u, g, p, fl) (0)
#define xfs_qm_dqattach(ip, fl) (0)
#define xfs_qm_dqattach_locked(ip, fl) (0)
#define xfs_qm_dqdetach(ip)
@@ -386,8 +390,8 @@ static inline int xfs_trans_reserve_quota_bydquots(struct xfs_trans *tp,
#define xfs_trans_unreserve_quota_nblks(tp, ip, nblks, ninos, flags) \
xfs_trans_reserve_quota_nblks(tp, ip, -(nblks), -(ninos), flags)
-#define xfs_trans_reserve_quota(tp, mp, ud, gd, nb, ni, f) \
- xfs_trans_reserve_quota_bydquots(tp, mp, ud, gd, nb, ni, \
+#define xfs_trans_reserve_quota(tp, mp, ud, gd, pd, nb, ni, f) \
+ xfs_trans_reserve_quota_bydquots(tp, mp, ud, gd, pd, nb, ni, \
f | XFS_QMOPT_RES_REGBLKS)
extern int xfs_qm_dqcheck(struct xfs_mount *, xfs_disk_dquot_t *,
diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
index f429d9d..8fd7894 100644
--- a/fs/xfs/xfs_sb.h
+++ b/fs/xfs/xfs_sb.h
@@ -140,6 +140,7 @@ typedef struct xfs_sb {
*/
xfs_ino_t sb_uquotino; /* user quota inode */
xfs_ino_t sb_gquotino; /* group quota inode */
+#define sb_pquotino sb_gquotino
__uint16_t sb_qflags; /* quota flags */
__uint8_t sb_flags; /* misc. flags */
__uint8_t sb_shared_vn; /* shared version number */
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 61ac734..595f5ac 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -538,14 +538,13 @@ xfs_showargs(
else if (mp->m_qflags & XFS_UQUOTA_ACCT)
seq_puts(m, "," MNTOPT_UQUOTANOENF);
- /* Either project or group quotas can be active, not both */
-
if (mp->m_qflags & XFS_PQUOTA_ACCT) {
if (mp->m_qflags & XFS_PQUOTA_ENFD)
seq_puts(m, "," MNTOPT_PRJQUOTA);
else
seq_puts(m, "," MNTOPT_PQUOTANOENF);
- } else if (mp->m_qflags & XFS_GQUOTA_ACCT) {
+ }
+ if (mp->m_qflags & XFS_GQUOTA_ACCT) {
if (mp->m_qflags & XFS_GQUOTA_ENFD)
seq_puts(m, "," MNTOPT_GRPQUOTA);
else
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 40460e8..fa4045f 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -113,7 +113,7 @@ xfs_trans_dup_dqinfo(
if(otp->t_flags & XFS_TRANS_DQ_DIRTY)
ntp->t_flags |= XFS_TRANS_DQ_DIRTY;
- for (j = 0; j < 2; j++) {
+ for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
if (oqa[i].qt_dquot == NULL)
break;
@@ -138,8 +138,13 @@ xfs_trans_dup_dqinfo(
oq->qt_ino_res = oq->qt_ino_res_used;
}
- oqa = otp->t_dqinfo->dqa_grpdquots;
- nqa = ntp->t_dqinfo->dqa_grpdquots;
+ if (oqa == otp->t_dqinfo->dqa_usrdquots) {
+ oqa = otp->t_dqinfo->dqa_grpdquots;
+ nqa = ntp->t_dqinfo->dqa_grpdquots;
+ } else {
+ oqa = otp->t_dqinfo->dqa_prjdquots;
+ nqa = ntp->t_dqinfo->dqa_prjdquots;
+ }
}
}
@@ -166,8 +171,10 @@ xfs_trans_mod_dquot_byino(
if (XFS_IS_UQUOTA_ON(mp) && ip->i_udquot)
(void) xfs_trans_mod_dquot(tp, ip->i_udquot, field, delta);
- if (XFS_IS_OQUOTA_ON(mp) && ip->i_gdquot)
+ if (XFS_IS_GQUOTA_ON(mp) && ip->i_gdquot)
(void) xfs_trans_mod_dquot(tp, ip->i_gdquot, field, delta);
+ if (XFS_IS_PQUOTA_ON(mp) && ip->i_pdquot)
+ (void) xfs_trans_mod_dquot(tp, ip->i_pdquot, field, delta);
}
STATIC xfs_dqtrx_t *
@@ -178,15 +185,20 @@ xfs_trans_get_dqtrx(
int i;
xfs_dqtrx_t *qa;
- qa = XFS_QM_ISUDQ(dqp) ?
- tp->t_dqinfo->dqa_usrdquots : tp->t_dqinfo->dqa_grpdquots;
+ if (XFS_QM_ISUDQ(dqp))
+ qa = tp->t_dqinfo->dqa_usrdquots;
+ else if (XFS_QM_ISGDQ(dqp))
+ qa = tp->t_dqinfo->dqa_grpdquots;
+ else if (XFS_QM_ISPDQ(dqp))
+ qa = tp->t_dqinfo->dqa_prjdquots;
+ else
+ return NULL;
for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
if (qa[i].qt_dquot == NULL ||
qa[i].qt_dquot == dqp)
return &qa[i];
}
-
return NULL;
}
@@ -340,9 +352,12 @@ xfs_trans_apply_dquot_deltas(
ASSERT(tp->t_dqinfo);
qa = tp->t_dqinfo->dqa_usrdquots;
- for (j = 0; j < 2; j++) {
+ for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
if (qa[0].qt_dquot == NULL) {
- qa = tp->t_dqinfo->dqa_grpdquots;
+ if (qa == tp->t_dqinfo->dqa_usrdquots)
+ qa = tp->t_dqinfo->dqa_grpdquots;
+ else
+ qa = tp->t_dqinfo->dqa_prjdquots;
continue;
}
@@ -496,9 +511,12 @@ xfs_trans_apply_dquot_deltas(
be64_to_cpu(dqp->q_core.d_rtbcount));
}
/*
- * Do the group quotas next
+ * Do the group quotas or project quotas next
*/
- qa = tp->t_dqinfo->dqa_grpdquots;
+ if (qa == tp->t_dqinfo->dqa_usrdquots)
+ qa = tp->t_dqinfo->dqa_grpdquots;
+ else
+ qa = tp->t_dqinfo->dqa_prjdquots;
}
}
@@ -523,7 +541,7 @@ xfs_trans_unreserve_and_mod_dquots(
qa = tp->t_dqinfo->dqa_usrdquots;
- for (j = 0; j < 2; j++) {
+ for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
qtrx = &qa[i];
/*
@@ -565,7 +583,10 @@ xfs_trans_unreserve_and_mod_dquots(
xfs_dqunlock(dqp);
}
- qa = tp->t_dqinfo->dqa_grpdquots;
+ if (qa == tp->t_dqinfo->dqa_usrdquots)
+ qa = tp->t_dqinfo->dqa_grpdquots;
+ else
+ qa = tp->t_dqinfo->dqa_prjdquots;
}
}
@@ -734,8 +755,8 @@ error_return:
/*
* Given dquot(s), make disk block and/or inode reservations against them.
- * The fact that this does the reservation against both the usr and
- * grp/prj quotas is important, because this follows a both-or-nothing
+ * The fact that this does the reservation against user, group and
+ * project quotas is important, because this follows a all-or-nothing
* approach.
*
* flags = XFS_QMOPT_FORCE_RES evades limit enforcement. Used by chown.
@@ -750,6 +771,7 @@ xfs_trans_reserve_quota_bydquots(
xfs_mount_t *mp,
xfs_dquot_t *udqp,
xfs_dquot_t *gdqp,
+ xfs_dquot_t *pdqp,
long nblks,
long ninos,
uint flags)
@@ -787,6 +809,24 @@ xfs_trans_reserve_quota_bydquots(
}
}
+ if (pdqp) {
+ error = xfs_trans_dqresv(tp, mp, pdqp, nblks, ninos, flags);
+ if (error) {
+ /*
+ * can't do it, so backout previous reservation
+ */
+ if (resvd) {
+ flags |= XFS_QMOPT_FORCE_RES;
+ xfs_trans_dqresv(tp, mp, udqp,
+ -nblks, -ninos, flags);
+ if (gdqp)
+ xfs_trans_dqresv(tp, mp, gdqp,
+ -nblks, -ninos, flags);
+ }
+ return error;
+ }
+ }
+
/*
* Didn't change anything critical, so, no need to log
*/
@@ -828,6 +868,7 @@ xfs_trans_reserve_quota_nblks(
*/
return xfs_trans_reserve_quota_bydquots(tp, mp,
ip->i_udquot, ip->i_gdquot,
+ ip->i_pdquot,
nblks, ninos, flags);
}
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index c22f4e0..e97f1f7 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -849,6 +849,7 @@ xfs_create(
prid_t prid;
struct xfs_dquot *udqp = NULL;
struct xfs_dquot *gdqp = NULL;
+ struct xfs_dquot *pdqp = NULL;
uint resblks;
uint log_res;
uint log_count;
@@ -867,7 +868,7 @@ xfs_create(
* Make sure that we have allocated dquot(s) on disk.
*/
error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid,
- XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);
+ XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp, &pdqp);
if (error)
return error;
@@ -919,7 +920,8 @@ xfs_create(
/*
* Reserve disk quota and the inode.
*/
- error = xfs_trans_reserve_quota(tp, mp, udqp, gdqp, resblks, 1, 0);
+ error = xfs_trans_reserve_quota(tp, mp, udqp, gdqp,
+ pdqp, resblks, 1, 0);
if (error)
goto out_trans_cancel;
@@ -983,7 +985,7 @@ xfs_create(
* These ids of the inode couldn't have changed since the new
* inode has been locked ever since it was created.
*/
- xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp);
+ xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp, pdqp);
error = xfs_bmap_finish(&tp, &free_list, &committed);
if (error)
@@ -995,6 +997,7 @@ xfs_create(
xfs_qm_dqrele(udqp);
xfs_qm_dqrele(gdqp);
+ xfs_qm_dqrele(pdqp);
*ipp = ip;
return 0;
@@ -1016,6 +1019,7 @@ xfs_create(
xfs_qm_dqrele(udqp);
xfs_qm_dqrele(gdqp);
+ xfs_qm_dqrele(pdqp);
if (unlock_dp_on_error)
xfs_iunlock(dp, XFS_ILOCK_EXCL);
@@ -1498,7 +1502,7 @@ xfs_symlink(
int n;
xfs_buf_t *bp;
prid_t prid;
- struct xfs_dquot *udqp, *gdqp;
+ struct xfs_dquot *udqp = NULL, *gdqp = NULL, *pdqp = NULL;
uint resblks;
*ipp = NULL;
@@ -1528,7 +1532,7 @@ xfs_symlink(
* Make sure that we have allocated dquot(s) on disk.
*/
error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid,
- XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);
+ XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp, &pdqp);
if (error)
goto std_return;
@@ -1569,7 +1573,8 @@ xfs_symlink(
/*
* Reserve disk quota : blocks and inode.
*/
- error = xfs_trans_reserve_quota(tp, mp, udqp, gdqp, resblks, 1, 0);
+ error = xfs_trans_reserve_quota(tp, mp, udqp, gdqp,
+ pdqp, resblks, 1, 0);
if (error)
goto error_return;
@@ -1607,7 +1612,7 @@ xfs_symlink(
/*
* Also attach the dquot(s) to it, if applicable.
*/
- xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp);
+ xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp, pdqp);
if (resblks)
resblks -= XFS_IALLOC_SPACE_RES(mp);
@@ -1691,6 +1696,7 @@ xfs_symlink(
error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
xfs_qm_dqrele(udqp);
xfs_qm_dqrele(gdqp);
+ xfs_qm_dqrele(pdqp);
*ipp = ip;
return 0;
@@ -1704,6 +1710,7 @@ xfs_symlink(
xfs_trans_cancel(tp, cancel_flags);
xfs_qm_dqrele(udqp);
xfs_qm_dqrele(gdqp);
+ xfs_qm_dqrele(pdqp);
if (unlock_dp_on_error)
xfs_iunlock(dp, XFS_ILOCK_EXCL);
@@ -2170,7 +2177,7 @@ xfs_free_file_space(
}
xfs_ilock(ip, XFS_ILOCK_EXCL);
error = xfs_trans_reserve_quota(tp, mp,
- ip->i_udquot, ip->i_gdquot,
+ ip->i_udquot, ip->i_gdquot, ip->i_pdquot,
resblks, 0, XFS_QMOPT_RES_REGBLKS);
if (error)
goto error1;
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC v6 PATCH 3/5] xfs: Add pquotaino to on-disk super block
2012-07-20 23:02 [RFC v6 PATCH 0/5] xfs: Allow pquota and gquota to be used together Chandra Seetharaman
2012-07-20 23:02 ` [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD Chandra Seetharaman
2012-07-20 23:02 ` [RFC v6 PATCH 2/5] xfs: Add pquota fields where gquota is used Chandra Seetharaman
@ 2012-07-20 23:02 ` Chandra Seetharaman
2012-08-15 2:09 ` Dave Chinner
2012-07-20 23:02 ` [RFC v6 PATCH 4/5] xfs: Add proper versioning support to fs_quota_stat Chandra Seetharaman
2012-07-20 23:03 ` [RFC v6 PATCH 5/5] xfs: Add a new field to fs_quota_stat to get pquota information Chandra Seetharaman
4 siblings, 1 reply; 19+ messages in thread
From: Chandra Seetharaman @ 2012-07-20 23:02 UTC (permalink / raw)
To: xfs; +Cc: Chandra Seetharaman
Add a new field to the superblock to add support for seperate pquota
with a specific version.
Define a macro to differentiate superblock with and without OQUOTA.
Keep backward compatibilty by alowing mount of older superblock
with no separate pquota inode.
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
fs/xfs/xfs_itable.c | 3 +-
fs/xfs/xfs_mount.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-
fs/xfs/xfs_qm.c | 18 +++++++++----
fs/xfs/xfs_qm_syscalls.c | 30 +++++++++++++++++-----
fs/xfs/xfs_quota.h | 8 ------
fs/xfs/xfs_sb.h | 20 ++++++++++++---
fs/xfs/xfs_super.c | 15 +++++++----
fs/xfs/xfs_trans_dquot.c | 4 ++-
include/linux/dqblk_xfs.h | 1 +
9 files changed, 123 insertions(+), 34 deletions(-)
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index eff577a..0fa60b4 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -42,7 +42,8 @@ xfs_internal_inum(
{
return (ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
(xfs_sb_version_hasquota(&mp->m_sb) &&
- (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino)));
+ (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino ||
+ ino == mp->m_sb.sb_pquotino)));
}
/*
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 22f23fd..992ec29 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -108,6 +108,7 @@ static const struct {
{ offsetof(xfs_sb_t, sb_logsunit), 0 },
{ offsetof(xfs_sb_t, sb_features2), 0 },
{ offsetof(xfs_sb_t, sb_bad_features2), 0 },
+ { offsetof(xfs_sb_t, sb_pquotino), 0 },
{ sizeof(xfs_sb_t), 0 }
};
@@ -618,6 +619,35 @@ 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);
+
+ if (xfs_sb_version_has_no_oquota(to)) {
+ if (to->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
+ xfs_notice(mp, "Super block has XFS_OQUOTA bits with "
+ "version NO_OQUOTA. Fixing it.\n");
+ to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
+ }
+ to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
+ } else {
+ if (to->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
+ XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
+ xfs_notice(mp, "Super block has XFS_[G|P]UOTA bits in "
+ "older version. Fixing it.\n");
+ to->sb_qflags &= ~(XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
+ XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD);
+ }
+ if (to->sb_qflags & XFS_OQUOTA_ENFD)
+ to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
+ XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
+ if (to->sb_qflags & XFS_OQUOTA_CHKD)
+ to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
+ XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
+ to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
+
+ if (to->sb_qflags & XFS_PQUOTA_ACCT) {
+ to->sb_pquotino = to->sb_gquotino;
+ to->sb_gquotino = NULLFSINO;
+ }
+ }
}
/*
@@ -637,11 +667,22 @@ xfs_sb_to_disk(
int first;
int size;
__uint16_t tmp16;
+ xfs_ino_t gquotino;
ASSERT(fields);
if (!fields)
return;
+ /*
+ * On-disk version earlier than NO_OQUOTA doesn't have sb_pquotino.
+ * so, we need to copy the value to gquotino field.
+ */
+ if (!xfs_sb_version_has_no_oquota(from) &&
+ (from->sb_qflags & (XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD)))
+ gquotino = from->sb_pquotino;
+ else
+ gquotino = from->sb_gquotino;
+
while (fields) {
f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
first = xfs_sb_info[f].offset;
@@ -651,7 +692,8 @@ xfs_sb_to_disk(
if (size == 1 || xfs_sb_info[f].type == 1) {
memcpy(to_ptr + first, from_ptr + first, size);
- } else if (f == XFS_SBS_QFLAGS) {
+ } else if ((f == XFS_SBS_QFLAGS) &&
+ !xfs_sb_version_has_no_oquota(from)) {
/*
* The in-core version of sb_qflags do not have
* XFS_OQUOTA_* flags, whereas the on-disk version
@@ -671,6 +713,8 @@ xfs_sb_to_disk(
tmp16 |= XFS_OQUOTA_CHKD;
*(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
+ } else if (f == XFS_SBS_GQUOTINO) {
+ *(__be64 *)(to_ptr + first) = cpu_to_be64(gquotino);
} else {
switch (size) {
case 2:
@@ -759,6 +803,12 @@ reread:
goto reread;
}
+ if (!xfs_sb_version_has_no_oquota(&mp->m_sb) &&
+ XFS_IS_PQUOTA_ON(mp)) {
+ mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
+ mp->m_sb.sb_gquotino = NULLFSINO;
+ }
+
/* Initialize per-cpu counters */
xfs_icsb_reinit_counters(mp);
@@ -1646,6 +1696,12 @@ xfs_mod_sb(xfs_trans_t *tp, __int64_t fields)
first = sizeof(xfs_sb_t);
last = 0;
+ if (!xfs_sb_version_has_no_oquota(&mp->m_sb) &&
+ XFS_IS_PQUOTA_ON(mp)) {
+ fields &= (__int64_t)~XFS_SB_PQUOTINO;
+ fields |= (__int64_t)XFS_SB_GQUOTINO;
+ }
+
/* translate/copy */
xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, fields);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 46c7e4c..3c61ce6 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -506,7 +506,8 @@ xfs_qm_need_dqattach(
if (!XFS_NOT_DQATTACHED(mp, ip))
return false;
if (ip->i_ino == mp->m_sb.sb_uquotino ||
- ip->i_ino == mp->m_sb.sb_gquotino)
+ ip->i_ino == mp->m_sb.sb_gquotino ||
+ ip->i_ino == mp->m_sb.sb_pquotino)
return false;
return true;
}
@@ -641,6 +642,7 @@ xfs_qm_dqdetach(
ASSERT(ip->i_ino != ip->i_mount->m_sb.sb_uquotino);
ASSERT(ip->i_ino != ip->i_mount->m_sb.sb_gquotino);
+ ASSERT(ip->i_ino != ip->i_mount->m_sb.sb_pquotino);
if (ip->i_udquot) {
xfs_qm_dqrele(ip->i_udquot);
ip->i_udquot = NULL;
@@ -838,19 +840,22 @@ xfs_qm_qino_alloc(
ASSERT((sbfields & (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
XFS_SB_GQUOTINO | XFS_SB_QFLAGS)) ==
(XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
- XFS_SB_GQUOTINO | XFS_SB_QFLAGS));
+ XFS_SB_GQUOTINO | XFS_SB_PQUOTINO | XFS_SB_QFLAGS));
xfs_sb_version_addquota(&mp->m_sb);
mp->m_sb.sb_uquotino = NULLFSINO;
mp->m_sb.sb_gquotino = NULLFSINO;
+ mp->m_sb.sb_pquotino = NULLFSINO;
/* qflags will get updated _after_ quotacheck */
mp->m_sb.sb_qflags = 0;
}
if (flags & XFS_QMOPT_UQUOTA)
mp->m_sb.sb_uquotino = (*ip)->i_ino;
- else
+ else if (flags & XFS_QMOPT_GQUOTA)
mp->m_sb.sb_gquotino = (*ip)->i_ino;
+ else
+ mp->m_sb.sb_pquotino = (*ip)->i_ino;
spin_unlock(&mp->m_sb_lock);
xfs_mod_sb(tp, sbfields);
@@ -1156,7 +1161,8 @@ xfs_qm_dqusage_adjust(
* rootino must have its resources accounted for, not so with the quota
* inodes.
*/
- if (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino) {
+ if (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino ||
+ ino == mp->m_sb.sb_pquotino) {
*res = BULKSTAT_RV_NOTHING;
return XFS_ERROR(EINVAL);
}
@@ -1455,7 +1461,7 @@ xfs_qm_init_quotainos(
} else {
flags |= XFS_QMOPT_SBVERSION;
sbflags |= (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
- XFS_SB_GQUOTINO | XFS_SB_QFLAGS);
+ XFS_SB_GQUOTINO | XFS_SB_PQUOTINO | XFS_SB_QFLAGS);
}
/*
@@ -1485,7 +1491,7 @@ xfs_qm_init_quotainos(
}
if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
error = xfs_qm_qino_alloc(mp, &pip,
- sbflags | XFS_SB_GQUOTINO,
+ sbflags | XFS_SB_PQUOTINO,
flags | XFS_QMOPT_PQUOTA);
if (error) {
if (uip)
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 509bbf7..f011cf3 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -200,8 +200,7 @@ xfs_qm_scall_quotaoff(
/*
* If quotas is completely disabled, close shop.
*/
- if (((flags & XFS_MOUNT_QUOTA_ALL) == XFS_MOUNT_QUOTA_SET1) ||
- ((flags & XFS_MOUNT_QUOTA_ALL) == XFS_MOUNT_QUOTA_SET2)) {
+ if ((flags & XFS_MOUNT_QUOTA_ALL) == XFS_MOUNT_QUOTA_ALL) {
mutex_unlock(&q->qi_quotaofflock);
xfs_qm_destroy_quotainfo(mp);
return (0);
@@ -296,8 +295,10 @@ xfs_qm_scall_trunc_qfiles(
if (flags & XFS_DQ_USER)
error = xfs_qm_scall_trunc_qfile(mp, mp->m_sb.sb_uquotino);
- if (flags & (XFS_DQ_GROUP|XFS_DQ_PROJ))
+ if (flags & XFS_DQ_GROUP)
error2 = xfs_qm_scall_trunc_qfile(mp, mp->m_sb.sb_gquotino);
+ if (flags & XFS_DQ_PROJ)
+ error2 = xfs_qm_scall_trunc_qfile(mp, mp->m_sb.sb_pquotino);
return error ? error : error2;
}
@@ -413,17 +414,18 @@ xfs_qm_scall_getqstat(
struct fs_quota_stat *out)
{
struct xfs_quotainfo *q = mp->m_quotainfo;
- struct xfs_inode *uip, *gip;
- boolean_t tempuqip, tempgqip;
+ struct xfs_inode *uip, *gip, *pip;
+ boolean_t tempuqip, tempgqip, temppqip;
- uip = gip = NULL;
- tempuqip = tempgqip = B_FALSE;
+ uip = gip = pip = NULL;
+ tempuqip = tempgqip = temppqip = B_FALSE;
memset(out, 0, sizeof(fs_quota_stat_t));
out->qs_version = FS_QSTAT_VERSION;
if (!xfs_sb_version_hasquota(&mp->m_sb)) {
out->qs_uquota.qfs_ino = NULLFSINO;
out->qs_gquota.qfs_ino = NULLFSINO;
+ out->qs_pquota.qfs_ino = NULLFSINO;
return (0);
}
out->qs_flags = (__uint16_t) xfs_qm_export_flags(mp->m_qflags &
@@ -432,10 +434,13 @@ xfs_qm_scall_getqstat(
out->qs_pad = 0;
out->qs_uquota.qfs_ino = mp->m_sb.sb_uquotino;
out->qs_gquota.qfs_ino = mp->m_sb.sb_gquotino;
+ if (&out->qs_gquota != &out->qs_pquota)
+ out->qs_pquota.qfs_ino = mp->m_sb.sb_pquotino;
if (q) {
uip = q->qi_uquotaip;
gip = q->qi_gquotaip;
+ pip = q->qi_pquotaip;
}
if (!uip && mp->m_sb.sb_uquotino != NULLFSINO) {
if (xfs_iget(mp, NULL, mp->m_sb.sb_uquotino,
@@ -447,6 +452,11 @@ xfs_qm_scall_getqstat(
0, 0, &gip) == 0)
tempgqip = B_TRUE;
}
+ if (!pip && mp->m_sb.sb_pquotino != NULLFSINO) {
+ if (xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
+ 0, 0, &pip) == 0)
+ temppqip = B_TRUE;
+ }
if (uip) {
out->qs_uquota.qfs_nblks = uip->i_d.di_nblocks;
out->qs_uquota.qfs_nextents = uip->i_d.di_nextents;
@@ -459,6 +469,12 @@ xfs_qm_scall_getqstat(
if (tempgqip)
IRELE(gip);
}
+ if (pip) {
+ out->qs_pquota.qfs_nblks = pip->i_d.di_nblocks;
+ out->qs_pquota.qfs_nextents = pip->i_d.di_nextents;
+ if (temppqip)
+ IRELE(pip);
+ }
if (q) {
out->qs_incoredqs = q->qi_dquots;
out->qs_btimelimit = q->qi_btimelimit;
diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
index 1c61c9b..6e218ba 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -278,14 +278,6 @@ typedef struct xfs_qoff_logformat {
(XFS_IS_PQUOTA_ON(mp) && \
(mp->m_sb.sb_qflags & XFS_PQUOTA_CHKD) == 0))
-#define XFS_MOUNT_QUOTA_SET1 (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD|\
- XFS_UQUOTA_CHKD|XFS_PQUOTA_ACCT|\
- XFS_PQUOTA_ENFD|XFS_PQUOTA_CHKD)
-
-#define XFS_MOUNT_QUOTA_SET2 (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD|\
- XFS_UQUOTA_CHKD|XFS_GQUOTA_ACCT|\
- XFS_GQUOTA_ENFD|XFS_GQUOTA_CHKD)
-
#define XFS_MOUNT_QUOTA_ALL (XFS_UQUOTA_ACCT|XFS_UQUOTA_ENFD|\
XFS_UQUOTA_CHKD|XFS_PQUOTA_ACCT|\
XFS_PQUOTA_ENFD|XFS_PQUOTA_CHKD|\
diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
index 8fd7894..7373108 100644
--- a/fs/xfs/xfs_sb.h
+++ b/fs/xfs/xfs_sb.h
@@ -81,11 +81,15 @@ struct xfs_mount;
#define XFS_SB_VERSION2_ATTR2BIT 0x00000008 /* Inline attr rework */
#define XFS_SB_VERSION2_PARENTBIT 0x00000010 /* parent pointers */
#define XFS_SB_VERSION2_PROJID32BIT 0x00000080 /* 32 bit project id */
+#define XFS_SB_VERSION2_NO_OQUOTA 0x00000100 /* No OQUOTA and *
+ * separate project *
+ * quota field */
#define XFS_SB_VERSION2_OKREALFBITS \
(XFS_SB_VERSION2_LAZYSBCOUNTBIT | \
XFS_SB_VERSION2_ATTR2BIT | \
- XFS_SB_VERSION2_PROJID32BIT)
+ XFS_SB_VERSION2_PROJID32BIT | \
+ XFS_SB_VERSION2_NO_OQUOTA)
#define XFS_SB_VERSION2_OKSASHFBITS \
(0)
#define XFS_SB_VERSION2_OKREALBITS \
@@ -140,7 +144,6 @@ typedef struct xfs_sb {
*/
xfs_ino_t sb_uquotino; /* user quota inode */
xfs_ino_t sb_gquotino; /* group quota inode */
-#define sb_pquotino sb_gquotino
__uint16_t sb_qflags; /* quota flags */
__uint8_t sb_flags; /* misc. flags */
__uint8_t sb_shared_vn; /* shared version number */
@@ -160,6 +163,7 @@ typedef struct xfs_sb {
* it for anything else.
*/
__uint32_t sb_bad_features2;
+ xfs_ino_t sb_pquotino; /* project quota inode */
/* must be padded to 64 bit alignment */
} xfs_sb_t;
@@ -230,6 +234,7 @@ typedef struct xfs_dsb {
* it for anything else.
*/
__be32 sb_bad_features2;
+ __be64 sb_pquotino; /* project quota inode */
/* must be padded to 64 bit alignment */
} xfs_dsb_t;
@@ -250,7 +255,7 @@ typedef enum {
XFS_SBS_GQUOTINO, XFS_SBS_QFLAGS, XFS_SBS_FLAGS, XFS_SBS_SHARED_VN,
XFS_SBS_INOALIGNMT, XFS_SBS_UNIT, XFS_SBS_WIDTH, XFS_SBS_DIRBLKLOG,
XFS_SBS_LOGSECTLOG, XFS_SBS_LOGSECTSIZE, XFS_SBS_LOGSUNIT,
- XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2,
+ XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2, XFS_SBS_PQUOTINO,
XFS_SBS_FIELDCOUNT
} xfs_sb_field_t;
@@ -276,6 +281,7 @@ typedef enum {
#define XFS_SB_FDBLOCKS XFS_SB_MVAL(FDBLOCKS)
#define XFS_SB_FEATURES2 XFS_SB_MVAL(FEATURES2)
#define XFS_SB_BAD_FEATURES2 XFS_SB_MVAL(BAD_FEATURES2)
+#define XFS_SB_PQUOTINO XFS_SB_MVAL(PQUOTINO)
#define XFS_SB_NUM_BITS ((int)XFS_SBS_FIELDCOUNT)
#define XFS_SB_ALL_BITS ((1LL << XFS_SB_NUM_BITS) - 1)
#define XFS_SB_MOD_BITS \
@@ -283,7 +289,7 @@ typedef enum {
XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO | XFS_SB_GQUOTINO | \
XFS_SB_QFLAGS | XFS_SB_SHARED_VN | XFS_SB_UNIT | XFS_SB_WIDTH | \
XFS_SB_ICOUNT | XFS_SB_IFREE | XFS_SB_FDBLOCKS | XFS_SB_FEATURES2 | \
- XFS_SB_BAD_FEATURES2)
+ XFS_SB_BAD_FEATURES2 | XFS_SB_PQUOTINO)
/*
@@ -504,6 +510,12 @@ static inline int xfs_sb_version_hasprojid32bit(xfs_sb_t *sbp)
(sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT);
}
+static inline int xfs_sb_version_has_no_oquota(xfs_sb_t *sbp)
+{
+ return xfs_sb_version_hasmorebits(sbp) &&
+ (sbp->sb_features2 & XFS_SB_VERSION2_NO_OQUOTA);
+}
+
/*
* end of superblock version macros
*/
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 595f5ac..b475057 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -399,12 +399,6 @@ xfs_parseargs(
}
#endif
- if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
- (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE))) {
- xfs_warn(mp, "cannot mount with both project and group quota");
- return EINVAL;
- }
-
if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
xfs_warn(mp, "sunit and swidth must be specified together");
return EINVAL;
@@ -1330,6 +1324,15 @@ xfs_fs_fill_super(
if (error)
goto out_destroy_counters;
+ if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
+ (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE)) &&
+ !xfs_sb_version_has_no_oquota(&mp->m_sb)) {
+ xfs_warn(mp, "Super block does not support "
+ "project and group quota together");
+ error = EINVAL;
+ goto out_free_sb;
+ }
+
error = xfs_finish_flags(mp);
if (error)
goto out_free_sb;
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index fa4045f..43f23e6 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -163,7 +163,8 @@ xfs_trans_mod_dquot_byino(
if (!XFS_IS_QUOTA_RUNNING(mp) ||
!XFS_IS_QUOTA_ON(mp) ||
ip->i_ino == mp->m_sb.sb_uquotino ||
- ip->i_ino == mp->m_sb.sb_gquotino)
+ ip->i_ino == mp->m_sb.sb_gquotino ||
+ ip->i_ino == mp->m_sb.sb_pquotino)
return;
if (tp->t_dqinfo == NULL)
@@ -856,6 +857,7 @@ xfs_trans_reserve_quota_nblks(
ASSERT(ip->i_ino != mp->m_sb.sb_uquotino);
ASSERT(ip->i_ino != mp->m_sb.sb_gquotino);
+ ASSERT(ip->i_ino != mp->m_sb.sb_pquotino);
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
ASSERT((flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
diff --git a/include/linux/dqblk_xfs.h b/include/linux/dqblk_xfs.h
index 8655280..f17e3bb 100644
--- a/include/linux/dqblk_xfs.h
+++ b/include/linux/dqblk_xfs.h
@@ -155,6 +155,7 @@ typedef struct fs_quota_stat {
__s8 qs_pad; /* unused */
fs_qfilestat_t qs_uquota; /* user quota storage information */
fs_qfilestat_t qs_gquota; /* group quota storage information */
+#define qs_pquota qs_gquota
__u32 qs_incoredqs; /* number of dquots incore */
__s32 qs_btimelimit; /* limit for blks timer */
__s32 qs_itimelimit; /* limit for inodes timer */
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC v6 PATCH 4/5] xfs: Add proper versioning support to fs_quota_stat
2012-07-20 23:02 [RFC v6 PATCH 0/5] xfs: Allow pquota and gquota to be used together Chandra Seetharaman
` (2 preceding siblings ...)
2012-07-20 23:02 ` [RFC v6 PATCH 3/5] xfs: Add pquotaino to on-disk super block Chandra Seetharaman
@ 2012-07-20 23:02 ` Chandra Seetharaman
2012-08-15 2:32 ` Dave Chinner
2012-07-20 23:03 ` [RFC v6 PATCH 5/5] xfs: Add a new field to fs_quota_stat to get pquota information Chandra Seetharaman
4 siblings, 1 reply; 19+ messages in thread
From: Chandra Seetharaman @ 2012-07-20 23:02 UTC (permalink / raw)
To: xfs; +Cc: Chandra Seetharaman
Add proper versioning support for fs_quota_stat.
Leave the earlier version as version 1 and add version 2 to add a new
field "fs_qfilestat_t qs_pquota"
Callers of the system call have to just set the version of the data
structure being passed, and kernel will fill as much data as possible.
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
fs/quota/quota.c | 6 +++++-
include/linux/dqblk_xfs.h | 28 +++++++++++++++++++++++++++-
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index 9a39120..ee929ca 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -203,8 +203,12 @@ static int quota_getxstate(struct super_block *sb, void __user *addr)
if (!sb->s_qcop->get_xstate)
return -ENOSYS;
+ if (copy_from_user(&fqs, addr, 1)) /* just get the version */
+ return -EFAULT;
+ if (!valid_qstat_version(fqs.qs_version))
+ fqs.qs_version = FS_QSTAT_VERSION;
ret = sb->s_qcop->get_xstate(sb, &fqs);
- if (!ret && copy_to_user(addr, &fqs, sizeof(fqs)))
+ if (!ret && copy_to_user(addr, &fqs, qstatsize(fqs.qs_version)))
return -EFAULT;
return ret;
}
diff --git a/include/linux/dqblk_xfs.h b/include/linux/dqblk_xfs.h
index f17e3bb..5be63fb 100644
--- a/include/linux/dqblk_xfs.h
+++ b/include/linux/dqblk_xfs.h
@@ -18,6 +18,7 @@
#define _LINUX_DQBLK_XFS_H
#include <linux/types.h>
+#include <linux/stddef.h>
/*
* Disk quota - quotactl(2) commands for the XFS Quota Manager (XQM).
@@ -139,6 +140,7 @@ typedef struct fs_disk_quota {
* incore.
*/
#define FS_QSTAT_VERSION 1 /* fs_quota_stat.qs_version */
+#define FS_QSTAT_VERSION_2 2 /* new field qs_pquota */
/*
* Some basic information about 'quota files'.
@@ -155,13 +157,37 @@ typedef struct fs_quota_stat {
__s8 qs_pad; /* unused */
fs_qfilestat_t qs_uquota; /* user quota storage information */
fs_qfilestat_t qs_gquota; /* group quota storage information */
-#define qs_pquota qs_gquota
__u32 qs_incoredqs; /* number of dquots incore */
__s32 qs_btimelimit; /* limit for blks timer */
__s32 qs_itimelimit; /* limit for inodes timer */
__s32 qs_rtbtimelimit;/* limit for rt blks timer */
__u16 qs_bwarnlimit; /* limit for num warnings */
__u16 qs_iwarnlimit; /* limit for num warnings */
+ fs_qfilestat_t qs_pquota; /* project quota storage information */
} fs_quota_stat_t;
+#define FS_QSTAT_V1_SIZE (offsetof(fs_quota_stat_t, qs_pquota))
+#define FS_QSTAT_V2_SIZE (FS_QSTAT_V1_SIZE + sizeof (fs_qfilestat_t))
+
+static inline int valid_qstat_version(int version)
+{
+ switch (version) {
+ case FS_QSTAT_VERSION:
+ case FS_QSTAT_VERSION_2:
+ return 1;
+ default:
+ return 0;
+ }
+}
+static inline int qstatsize(int version)
+{
+ switch (version) {
+ case FS_QSTAT_VERSION_2:
+ return FS_QSTAT_V2_SIZE;
+ case FS_QSTAT_VERSION:
+ default:
+ return FS_QSTAT_V1_SIZE;
+ }
+}
+
#endif /* _LINUX_DQBLK_XFS_H */
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC v6 PATCH 5/5] xfs: Add a new field to fs_quota_stat to get pquota information
2012-07-20 23:02 [RFC v6 PATCH 0/5] xfs: Allow pquota and gquota to be used together Chandra Seetharaman
` (3 preceding siblings ...)
2012-07-20 23:02 ` [RFC v6 PATCH 4/5] xfs: Add proper versioning support to fs_quota_stat Chandra Seetharaman
@ 2012-07-20 23:03 ` Chandra Seetharaman
2012-08-15 2:37 ` Dave Chinner
4 siblings, 1 reply; 19+ messages in thread
From: Chandra Seetharaman @ 2012-07-20 23:03 UTC (permalink / raw)
To: xfs; +Cc: Chandra Seetharaman
Use separate structure for filling the project quota information
for the Q_XGETQSTAT quota command.
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
fs/xfs/xfs_qm_syscalls.c | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index f011cf3..482efc0 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -421,7 +421,6 @@ xfs_qm_scall_getqstat(
tempuqip = tempgqip = temppqip = B_FALSE;
memset(out, 0, sizeof(fs_quota_stat_t));
- out->qs_version = FS_QSTAT_VERSION;
if (!xfs_sb_version_hasquota(&mp->m_sb)) {
out->qs_uquota.qfs_ino = NULLFSINO;
out->qs_gquota.qfs_ino = NULLFSINO;
@@ -434,8 +433,6 @@ xfs_qm_scall_getqstat(
out->qs_pad = 0;
out->qs_uquota.qfs_ino = mp->m_sb.sb_uquotino;
out->qs_gquota.qfs_ino = mp->m_sb.sb_gquotino;
- if (&out->qs_gquota != &out->qs_pquota)
- out->qs_pquota.qfs_ino = mp->m_sb.sb_pquotino;
if (q) {
uip = q->qi_uquotaip;
@@ -452,11 +449,6 @@ xfs_qm_scall_getqstat(
0, 0, &gip) == 0)
tempgqip = B_TRUE;
}
- if (!pip && mp->m_sb.sb_pquotino != NULLFSINO) {
- if (xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
- 0, 0, &pip) == 0)
- temppqip = B_TRUE;
- }
if (uip) {
out->qs_uquota.qfs_nblks = uip->i_d.di_nblocks;
out->qs_uquota.qfs_nextents = uip->i_d.di_nextents;
@@ -469,11 +461,19 @@ xfs_qm_scall_getqstat(
if (tempgqip)
IRELE(gip);
}
- if (pip) {
- out->qs_pquota.qfs_nblks = pip->i_d.di_nblocks;
- out->qs_pquota.qfs_nextents = pip->i_d.di_nextents;
- if (temppqip)
- IRELE(pip);
+ if (out->qs_version >= FS_QSTAT_VERSION_2) {
+ out->qs_pquota.qfs_ino = mp->m_sb.sb_pquotino;
+ if (!pip && mp->m_sb.sb_pquotino != NULLFSINO) {
+ if (xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
+ 0, 0, &pip) == 0)
+ temppqip = B_TRUE;
+ }
+ if (pip) {
+ out->qs_pquota.qfs_nblks = pip->i_d.di_nblocks;
+ out->qs_pquota.qfs_nextents = pip->i_d.di_nextents;
+ if (temppqip)
+ IRELE(pip);
+ }
}
if (q) {
out->qs_incoredqs = q->qi_dquots;
--
1.7.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD
2012-07-20 23:02 ` [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD Chandra Seetharaman
@ 2012-08-14 22:46 ` Dave Chinner
2012-08-22 22:53 ` Chandra Seetharaman
0 siblings, 1 reply; 19+ messages in thread
From: Dave Chinner @ 2012-08-14 22:46 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
On Fri, Jul 20, 2012 at 06:02:08PM -0500, Chandra Seetharaman wrote:
> Remove all incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD. Instead,
> start using XFS_GQUOTA_.* XFS_PQUOTA_.* counterparts for GQUOTA nd
> PQUOTA respectively.
>
> No changes are made to the on-disk version of the superblock yet. On-disk
> copy still uses XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD.
>
> Read and write of the superblock does the conversion from *OQUOTA*
> to *[PG]QUOTA*.
Couple of minor style things....
> @@ -622,6 +636,7 @@ xfs_sb_to_disk(
> xfs_sb_field_t f;
> int first;
> int size;
> + __uint16_t tmp16;
tmp16 is a bad name for a temporary variable. Move it to the scope
that uses it, and name it for it's purpose. i.e:
>
> ASSERT(fields);
> if (!fields)
> @@ -636,6 +651,26 @@ xfs_sb_to_disk(
>
> if (size == 1 || xfs_sb_info[f].type == 1) {
> memcpy(to_ptr + first, from_ptr + first, size);
> + } else if (f == XFS_SBS_QFLAGS) {
__uint16_t qflags;
> + /*
> + * The in-core version of sb_qflags do not have
> + * XFS_OQUOTA_* flags, whereas the on-disk version
> + * does. Save the in-core sb_qflags temporarily,
> + * removing the new XFS_{PG}QUOTA_* flags and re-apply
> + * the old on-disk flags.
> + */
> + tmp16 = from->sb_qflags &
qflags = from->sb_qflags & ....
> + ~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
> + XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
> +
> + if (from->sb_qflags &
> + (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
> + tmp16 |= XFS_OQUOTA_ENFD;
> + if (from->sb_qflags &
> + (XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
> + tmp16 |= XFS_OQUOTA_CHKD;
> +
> + *(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
> } else {
> switch (size) {
> case 2:
....
> @@ -339,9 +339,11 @@ xfs_qm_scall_quotaon(
> ||
> ((flags & XFS_PQUOTA_ACCT) == 0 &&
> (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT) == 0 &&
> - (flags & XFS_GQUOTA_ACCT) == 0 &&
> + (flags & XFS_PQUOTA_ENFD))
> + ||
> + ((flags & XFS_GQUOTA_ACCT) == 0 &&
> (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT) == 0 &&
> - (flags & XFS_OQUOTA_ENFD))) {
> + (flags & XFS_GQUOTA_ENFD))) {
Can you fix the flat indenting here at the same time so that the
logic is obvious at a glance and consistent with the rest of the XFS
code? i.e.
if (((flags & XFS_UQUOTA_ACCT) == 0 &&
(mp->m_sb.sb_qflags & XFS_UQUOTA_ACCT) == 0 &&
(flags & XFS_UQUOTA_ENFD)) ||
((flags & XFS_PQUOTA_ACCT) == 0 &&
(mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT) == 0 &&
(flags & XFS_PQUOTA_ENFD)) ||
(flags & XFS_GQUOTA_ACCT) == 0 &&
(mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT) == 0 &&
(flags & XFS_GQUOTA_ENFD))) {
> xfs_debug(mp,
> "%s: Can't enforce without acct, flags=%x sbflags=%x\n",
> __func__, flags, mp->m_sb.sb_qflags);
> @@ -771,8 +773,10 @@ xfs_qm_scall_getquota(
> * so return zeroes in that case.
> */
> if ((!XFS_IS_UQUOTA_ENFORCED(mp) && dqp->q_core.d_flags == XFS_DQ_USER) ||
> - (!XFS_IS_OQUOTA_ENFORCED(mp) &&
> - (dqp->q_core.d_flags & (XFS_DQ_PROJ | XFS_DQ_GROUP)))) {
> + (!XFS_IS_PQUOTA_ENFORCED(mp)
> + && dqp->q_core.d_flags == XFS_DQ_PROJ) ||
> + (!XFS_IS_GQUOTA_ENFORCED(mp)
> + && dqp->q_core.d_flags == XFS_DQ_GROUP)) {
Same here:
if ((!XFS_IS_UQUOTA_ENFORCED(mp) &&
dqp->q_core.d_flags == XFS_DQ_USER) ||
(!XFS_IS_PQUOTA_ENFORCED(mp) &&
dqp->q_core.d_flags == XFS_DQ_PROJ) ||
(!XFS_IS_GQUOTA_ENFORCED(mp) &&
dqp->q_core.d_flags == XFS_DQ_GROUP)) {
Otherwise it looks good. Fix the above and you can add a
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 2/5] xfs: Add pquota fields where gquota is used.
2012-07-20 23:02 ` [RFC v6 PATCH 2/5] xfs: Add pquota fields where gquota is used Chandra Seetharaman
@ 2012-08-15 1:15 ` Dave Chinner
2012-08-22 22:56 ` Chandra Seetharaman
0 siblings, 1 reply; 19+ messages in thread
From: Dave Chinner @ 2012-08-15 1:15 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
On Fri, Jul 20, 2012 at 06:02:25PM -0500, Chandra Seetharaman wrote:
> Add project quota changes to all the places where group quota field
> is used:
> * add separate project quota members into various structures
> * split project quota and group quotas so that instead of overriding
> the group quota members incore, the new project quota members are
> used instead
> * get rid of usage of the OQUOTA flag incore, in favor of separate group
> and project quota flags.
> * add a project dquot argument to various functions.
>
> No externally visible interfaces changed and no superblock changes yet.
Lots of comments below.
>
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> ---
> fs/xfs/xfs_dquot.c | 16 ++-
> fs/xfs/xfs_dquot.h | 11 ++-
> fs/xfs/xfs_iget.c | 2 +-
> fs/xfs/xfs_inode.h | 1 +
> fs/xfs/xfs_ioctl.c | 14 ++--
> fs/xfs/xfs_iops.c | 4 +-
> fs/xfs/xfs_qm.c | 255 ++++++++++++++++++++++++++++++++--------------
> fs/xfs/xfs_qm.h | 15 ++--
> fs/xfs/xfs_qm_bhv.c | 2 +-
> fs/xfs/xfs_qm_syscalls.c | 19 +++-
> fs/xfs/xfs_quota.h | 38 ++++---
> fs/xfs/xfs_sb.h | 1 +
> fs/xfs/xfs_super.c | 5 +-
> fs/xfs/xfs_trans_dquot.c | 71 ++++++++++---
> fs/xfs/xfs_vnodeops.c | 23 +++--
> 15 files changed, 326 insertions(+), 151 deletions(-)
>
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index bf27fcc..42b8b79 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -751,7 +751,7 @@ xfs_qm_dqput_final(
> struct xfs_dquot *dqp)
> {
> struct xfs_quotainfo *qi = dqp->q_mount->m_quotainfo;
> - struct xfs_dquot *gdqp;
> + struct xfs_dquot *gdqp, *pdqp;
We tend not to declare multiple variables on the one line - just add
a new line with the new variable.
>
> trace_xfs_dqput_free(dqp);
>
> @@ -765,21 +765,29 @@ xfs_qm_dqput_final(
>
> /*
> * If we just added a udquot to the freelist, then we want to release
> - * the gdquot reference that it (probably) has. Otherwise it'll keep
> - * the gdquot from getting reclaimed.
> + * the gdquot/pdquot reference that it (probably) has. Otherwise it'll
> + * keep the gdquot/pdquot from getting reclaimed.
> */
> gdqp = dqp->q_gdquot;
> if (gdqp) {
> xfs_dqlock(gdqp);
> dqp->q_gdquot = NULL;
> }
> +
> + pdqp = dqp->q_pdquot;
> + if (pdqp) {
> + xfs_dqlock(pdqp);
> + dqp->q_pdquot = NULL;
> + }
seeing this (and the various dupilications in this patch) makes me
wonder if we'd be better off with array based abstractions and
loops. That's not important for this patch set, but if we ever want
to add another type of quota, then it would make sense...
> diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> index 7d20af2..893cd5e 100644
> --- a/fs/xfs/xfs_dquot.h
> +++ b/fs/xfs/xfs_dquot.h
> @@ -46,6 +46,7 @@ typedef struct xfs_dquot {
> xfs_fileoff_t q_fileoffset; /* offset in quotas file */
>
> struct xfs_dquot*q_gdquot; /* group dquot, hint only */
> + struct xfs_dquot *q_pdquot; /* project dquot, hint only */
You may as well put a space in the q_gdquot declaration so they are
consistent....
> + return ip->i_pdquot;
> default:
> return NULL;
> }
> @@ -136,7 +139,9 @@ static inline xfs_dquot_t *xfs_inode_dquot(struct xfs_inode *ip, int type)
> #define XFS_DQ_TO_QINF(dqp) ((dqp)->q_mount->m_quotainfo)
> #define XFS_DQ_TO_QIP(dqp) (XFS_QM_ISUDQ(dqp) ? \
> XFS_DQ_TO_QINF(dqp)->qi_uquotaip : \
> - XFS_DQ_TO_QINF(dqp)->qi_gquotaip)
> + (XFS_QM_ISGDQ(dqp) ? \
> + XFS_DQ_TO_QINF(dqp)->qi_gquotaip : \
> + XFS_DQ_TO_QINF(dqp)->qi_pquotaip))
nested ?: constructs are a bit nasty. Perhaps this should be
converted to a static inline function like:
static inline struct xfs_inode *
xfs_dq_to_quota_inode(
struct xfs_dquot *dqp)
{
if (XFS_QM_ISUDQ(dqp))
return dqp->q_mount->m_quotainfo->qi_uquotaip;
if (XFS_QM_ISGDQ(dqp))
return dqp->q_mount->m_quotainfo->qi_gquotaip;
ASSERT(XFS_QM_ISPDQ(dqp)));
return dqp->q_mount->m_quotainfo->qi_pquotaip;
}
>
> extern int xfs_qm_dqread(struct xfs_mount *, xfs_dqid_t, uint,
> uint, struct xfs_dquot **);
> diff --git a/fs/xfs/xfs_iget.c b/fs/xfs/xfs_iget.c
> index 1bb4365..e97fb18 100644
> --- a/fs/xfs/xfs_iget.c
> +++ b/fs/xfs/xfs_iget.c
> @@ -346,7 +346,7 @@ xfs_iget_cache_miss(
> iflags = XFS_INEW;
> if (flags & XFS_IGET_DONTCACHE)
> iflags |= XFS_IDONTCACHE;
> - ip->i_udquot = ip->i_gdquot = NULL;
> + ip->i_udquot = ip->i_gdquot = ip->i_pdquot = NULL;
That's getting a little messy. I think you should convert these to
an assignment per line. i.e:
ip->i_udquot = NULL;
ip->i_gdquot = NULL;
ip->i_pdquot = NULL;
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 3fb3730..46c7e4c 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -134,7 +134,7 @@ xfs_qm_dqpurge(
> {
> struct xfs_mount *mp = dqp->q_mount;
> struct xfs_quotainfo *qi = mp->m_quotainfo;
> - struct xfs_dquot *gdqp = NULL;
> + struct xfs_dquot *gdqp = NULL, *pdqp = NULL;
New variable on a new line.
>
> xfs_dqlock(dqp);
> if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) {
> @@ -143,8 +143,8 @@ xfs_qm_dqpurge(
> }
>
> /*
> - * If this quota has a group hint attached, prepare for releasing it
> - * now.
> + * If this quota has a group/project hint attached, prepare for
> + * releasing it now.
I'd just say "If this quota has a hint attached..."
> /*
> - * Given a udquot and gdquot, attach a ptr to the group dquot in the
> + * Given a udquot and gdquot, attach a ptr to the group/project dquot in the
> * udquot as a hint for future lookups.
> */
> STATIC void
> -xfs_qm_dqattach_grouphint(
> - xfs_dquot_t *udq,
> - xfs_dquot_t *gdq)
> +xfs_qm_dqattach_grouphint(xfs_inode_t *ip, int type)
> {
> - xfs_dquot_t *tmp;
> + xfs_dquot_t **tmp, *gpdq, *tmp1, *udq = ip->i_udquot;
Don't use typedefs for new definitions, tmp is a really bad name,
and use consistent style:
STATIC void
xfs_qm_dqattach_grouphint(
struct xfs_inode *ip,
int type)
{
struct xfs_dquot *udq = ip->i_udquot;
struct xfs_dquot *gpdq;
struct xfs_dquot **dqhint;
struct xfs_dquot *tmp1;
> + gpdq = (type == XFS_DQ_GROUP) ? ip->i_gdquot : ip->i_pdquot;
> xfs_dqlock(udq);
>
> - tmp = udq->q_gdquot;
> - if (tmp) {
> - if (tmp == gdq)
> + tmp = (type == XFS_DQ_GROUP) ? &udq->q_gdquot : &udq->q_pdquot;
if (type == XFS_DQ_GROUP) {
gpdq = ip->i_gdquot;
dqhint = &udq->q_gdquot;
} else {
gpdq = ip->i_gdquot;
dqhint = &udq->q_gdquot;
}
> +
> + if (*tmp) {
> + if (*tmp == gpdq)
> goto done;
>
> - udq->q_gdquot = NULL;
> - xfs_qm_dqrele(tmp);
> + tmp1 = *tmp;
> + *tmp = NULL;
> + xfs_qm_dqrele(tmp1);
> }
>
> - udq->q_gdquot = xfs_qm_dqhold(gdq);
> + *tmp = xfs_qm_dqhold(gpdq);
if (*dqhint) {
struct xfs_dquot *tmp;
if (*dqhint == gpdq)
goto done;
tmp = *dqhint;
*dqhint = NULL;
xfs_qm_rele(tmp);
}
*dqhint = xfs_qm_dqhold(gpdq);
And when we get rid of the tmp names, all of a sudden the code goes
from being unreadable to being obviously suboptimal. i.e:
if (*dqhint == gpdq)
goto done;
if (*dqhint)
xfs_qm_rele(*dqhint);
*dqhint = xfs_qm_dqhold(gpdq);
We don't need the second temp variable because we have the dquot
locked and so nobody is going to be accessing the hint in the dquot
after we've released it. If they are accessing it unlocked, then it
is already racy and setting the dqhint to null doesn't change
anything....
> @@ -1227,7 +1269,7 @@ xfs_qm_quotacheck(
> int done, count, error, error2;
> xfs_ino_t lastino;
> size_t structsz;
> - xfs_inode_t *uip, *gip;
> + xfs_inode_t *uip, *gip, *pip;
struct xfs_inode *uip = mp->m_quotainfo->qi_uquotaip;
struct xfs_inode *gip = mp->m_quotainfo->qi_gquotaip;
struct xfs_inode *pip = mp->m_quotainfo->qi_pquotaip;
> uint flags;
> LIST_HEAD (buffer_list);
>
> @@ -1236,7 +1278,8 @@ xfs_qm_quotacheck(
> lastino = 0;
> flags = 0;
>
> - ASSERT(mp->m_quotainfo->qi_uquotaip || mp->m_quotainfo->qi_gquotaip);
> + ASSERT(mp->m_quotainfo->qi_uquotaip || mp->m_quotainfo->qi_gquotaip
> + || mp->m_quotainfo->qi_pquotaip);
ASSERT(uip || gip || pip);
> ASSERT(XFS_IS_QUOTA_RUNNING(mp));
>
> xfs_notice(mp, "Quotacheck needed: Please wait.");
> @@ -1257,13 +1300,20 @@ xfs_qm_quotacheck(
>
> gip = mp->m_quotainfo->qi_gquotaip;
> if (gip) {
> - error = xfs_qm_dqiterate(mp, gip, XFS_IS_GQUOTA_ON(mp) ?
> - XFS_QMOPT_GQUOTA : XFS_QMOPT_PQUOTA,
> + error = xfs_qm_dqiterate(mp, gip, XFS_QMOPT_GQUOTA,
> &buffer_list);
> if (error)
> goto error_return;
> - flags |= XFS_IS_GQUOTA_ON(mp) ?
> - XFS_GQUOTA_CHKD : XFS_PQUOTA_CHKD;
> + flags |= XFS_GQUOTA_CHKD;
> + }
> +
> + pip = mp->m_quotainfo->qi_pquotaip;
> + if (pip) {
> + error = xfs_qm_dqiterate(mp, pip, XFS_QMOPT_PQUOTA,
> + &buffer_list);
> + if (error)
> + goto error_return;
> + flags |= XFS_PQUOTA_CHKD;
> }
>
> do {
> @@ -1358,13 +1408,13 @@ STATIC int
> xfs_qm_init_quotainos(
> xfs_mount_t *mp)
> {
> - xfs_inode_t *uip, *gip;
> + xfs_inode_t *uip, *gip, *pip;
> int error;
> __int64_t sbflags;
> uint flags;
>
> ASSERT(mp->m_quotainfo);
> - uip = gip = NULL;
> + uip = gip = pip = NULL;
Same as above.
> sbflags = 0;
> flags = 0;
>
> @@ -1379,7 +1429,7 @@ xfs_qm_init_quotainos(
> 0, 0, &uip)))
> return XFS_ERROR(error);
> }
> - if (XFS_IS_OQUOTA_ON(mp) &&
> + if (XFS_IS_GQUOTA_ON(mp) &&
> mp->m_sb.sb_gquotino != NULLFSINO) {
> ASSERT(mp->m_sb.sb_gquotino > 0);
> if ((error = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino,
> @@ -1389,6 +1439,19 @@ xfs_qm_init_quotainos(
> return XFS_ERROR(error);
> }
> }
> + if (XFS_IS_PQUOTA_ON(mp) &&
> + mp->m_sb.sb_pquotino != NULLFSINO) {
> + ASSERT(mp->m_sb.sb_pquotino > 0);
> + error = xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
> + 0, 0, &pip);
> + if (error) {
> + if (uip)
> + IRELE(uip);
> + if (gip)
> + IRELE(gip);
> + return XFS_ERROR(error);
> + }
This error handling is repeated a couple of times. It is better to
use an error stack at the end of the function for this. i.e.
if (error)
goto error_rele;
...
return 0;
error_rele:
if (uip)
IRELE(uip);
if (gip)
IRELE(gip);
if (pip)
IRELE(pip);
return XFS_ERROR(error);
}
> @@ -1621,10 +1697,11 @@ xfs_qm_vop_dqalloc(
> prid_t prid,
> uint flags,
> struct xfs_dquot **O_udqpp,
> - struct xfs_dquot **O_gdqpp)
> + struct xfs_dquot **O_gdqpp,
> + struct xfs_dquot **O_pdqpp)
> {
> struct xfs_mount *mp = ip->i_mount;
> - struct xfs_dquot *uq, *gq;
> + struct xfs_dquot *uq, *gq, *pq;
Separate lines with initialisation...
> int error;
> uint lockflags;
>
> @@ -1649,7 +1726,7 @@ xfs_qm_vop_dqalloc(
> }
> }
>
> - uq = gq = NULL;
> + uq = gq = pq = NULL;
So this can be killed.
> if ((flags & XFS_QMOPT_UQUOTA) && XFS_IS_UQUOTA_ON(mp)) {
> if (ip->i_d.di_uid != uid) {
> /*
> @@ -1705,25 +1782,28 @@ xfs_qm_vop_dqalloc(
> ASSERT(ip->i_gdquot);
> gq = xfs_qm_dqhold(ip->i_gdquot);
> }
> - } else if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
> + }
> + if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
> if (xfs_get_projid(ip) != prid) {
> xfs_iunlock(ip, lockflags);
> if ((error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)prid,
> XFS_DQ_PROJ,
> XFS_QMOPT_DQALLOC |
> XFS_QMOPT_DOWARN,
> - &gq))) {
> + &pq))) {
Separate the function call fro the if() statement. Also, both dqid_t
and prid_t are uint32_t, so there is no need for a cast:
error = xfs_qm_dqget(mp, NULL, prid, XFS_DQ_PROJ,
XFS_QMOPT_DQALLOC | XFS_QMOPT_DOWARN,
&gq);
if (error) {
....
> @@ -1790,11 +1874,13 @@ xfs_qm_vop_chown_reserve(
> xfs_inode_t *ip,
> xfs_dquot_t *udqp,
> xfs_dquot_t *gdqp,
> + xfs_dquot_t *pdqp,
Probably should remove the typedefs while adding new parameters.
> uint flags)
> {
> xfs_mount_t *mp = ip->i_mount;
> uint delblks, blkflags, prjflags = 0;
> - xfs_dquot_t *unresudq, *unresgdq, *delblksudq, *delblksgdq;
> + xfs_dquot_t *unresudq, *unresgdq, *unrespdq;
> + xfs_dquot_t *delblksudq, *delblksgdq, *delblkspdq;
> int error;
Same here, and use one variable per line with initialisation.
>
>
> @@ -1802,7 +1888,8 @@ xfs_qm_vop_chown_reserve(
> ASSERT(XFS_IS_QUOTA_RUNNING(mp));
>
> delblks = ip->i_delayed_blks;
> - delblksudq = delblksgdq = unresudq = unresgdq = NULL;
> + delblksudq = delblksgdq = delblkspdq = NULL;
> + unresudq = unresgdq = unrespdq = NULL;
So this can be removed.
> blkflags = XFS_IS_REALTIME_INODE(ip) ?
> XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS;
>
> @@ -1819,25 +1906,28 @@ xfs_qm_vop_chown_reserve(
> unresudq = ip->i_udquot;
> }
> }
> - if (XFS_IS_OQUOTA_ON(ip->i_mount) && gdqp) {
> - if (XFS_IS_PQUOTA_ON(ip->i_mount) &&
> - xfs_get_projid(ip) != be32_to_cpu(gdqp->q_core.d_id))
> - prjflags = XFS_QMOPT_ENOSPC;
> -
> - if (prjflags ||
> - (XFS_IS_GQUOTA_ON(ip->i_mount) &&
> - ip->i_d.di_gid != be32_to_cpu(gdqp->q_core.d_id))) {
> - delblksgdq = gdqp;
> - if (delblks) {
> - ASSERT(ip->i_gdquot);
> - unresgdq = ip->i_gdquot;
> - }
> + if (XFS_IS_GQUOTA_ON(ip->i_mount) && gdqp &&
> + ip->i_d.di_gid != be32_to_cpu(gdqp->q_core.d_id)) {
> + delblksgdq = gdqp;
> + if (delblks) {
> + ASSERT(ip->i_gdquot);
> + unresgdq = ip->i_gdquot;
> + }
> + }
> +
> + if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
> + xfs_get_projid(ip) != be32_to_cpu(pdqp->q_core.d_id)) {
> + prjflags = XFS_QMOPT_ENOSPC;
> + delblkspdq = pdqp;
> + if (delblks) {
> + ASSERT(ip->i_pdquot);
> + unrespdq = ip->i_pdquot;
> }
> }
>
> if ((error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
> - delblksudq, delblksgdq, ip->i_d.di_nblocks, 1,
> - flags | blkflags | prjflags)))
> + delblksudq, delblksgdq, delblkspdq, ip->i_d.di_nblocks,
> + 1, flags | blkflags | prjflags)))
Separate the function call from the if().
> return (error);
>
> /*
> @@ -1850,15 +1940,16 @@ xfs_qm_vop_chown_reserve(
> /*
> * Do the reservations first. Unreservation can't fail.
> */
> - ASSERT(delblksudq || delblksgdq);
> - ASSERT(unresudq || unresgdq);
> + ASSERT(delblksudq || delblksgdq || delblkspdq);
> + ASSERT(unresudq || unresgdq || unrespdq);
> if ((error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
> - delblksudq, delblksgdq, (xfs_qcnt_t)delblks, 0,
> + delblksudq, delblksgdq, delblkspdq,
> + (xfs_qcnt_t)delblks, 0,
> flags | blkflags | prjflags)))
Same again.
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 44b858b..256ff71 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -44,9 +44,11 @@ extern struct kmem_zone *xfs_qm_dqtrxzone;
> typedef struct xfs_quotainfo {
> struct radix_tree_root qi_uquota_tree;
> struct radix_tree_root qi_gquota_tree;
> + struct radix_tree_root qi_pquota_tree;
> struct mutex qi_tree_lock;
> xfs_inode_t *qi_uquotaip; /* user quota inode */
> xfs_inode_t *qi_gquotaip; /* group quota inode */
> + xfs_inode_t *qi_pquotaip; /* project quota inode */
convert to struct xfs_inode.
> struct list_head qi_lru_list;
> struct mutex qi_lru_lock;
> int qi_lru_count;
> @@ -70,26 +72,25 @@ typedef struct xfs_quotainfo {
> } xfs_quotainfo_t;
>
> #define XFS_DQUOT_TREE(qi, type) \
> - ((type & XFS_DQ_USER) ? \
> - &((qi)->qi_uquota_tree) : \
> - &((qi)->qi_gquota_tree))
> + ((type & XFS_DQ_USER) ? &((qi)->qi_uquota_tree) : \
> + ((type & XFS_DQ_GROUP) ? &((qi)->qi_gquota_tree) : \
> + &((qi)->qi_pquota_tree)))
Convert to static inline, I think.
static inline struct radix_tree_root *
xfs_dquot_tree(
struct xfs_quotainfo *qi,
int type)
{
switch(type) {
case XFS_DQ_USER:
return qi->qi_uquota_tree;
case XFS_DQ_GROUP:
return qi->qi_gquota_tree;
case XFS_DQ_PROJ:
return qi->qi_pquota_tree;
default:
ASSERT(0);
}
return NULL;
}
> @@ -267,8 +265,10 @@ typedef struct xfs_qoff_logformat {
> */
> #define XFS_NOT_DQATTACHED(mp, ip) ((XFS_IS_UQUOTA_ON(mp) &&\
> (ip)->i_udquot == NULL) || \
> - (XFS_IS_OQUOTA_ON(mp) && \
> - (ip)->i_gdquot == NULL))
> + (XFS_IS_GQUOTA_ON(mp) && \
> + (ip)->i_gdquot == NULL) || \
> + (XFS_IS_PQUOTA_ON(mp) && \
> + (ip)->i_pdquot == NULL))
#define XFS_NOT_DQATTACHED(mp, ip) \
((XFS_IS_UQUOTA_ON(mp) && (ip)->i_udquot == NULL) || \
(XFS_IS_GQUOTA_ON(mp) && (ip)->i_gdquot == NULL) || \
(XFS_IS_PQUOTA_ON(mp) && (ip)->i_pdquot == NULL))
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -113,7 +113,7 @@ xfs_trans_dup_dqinfo(
> if(otp->t_flags & XFS_TRANS_DQ_DIRTY)
> ntp->t_flags |= XFS_TRANS_DQ_DIRTY;
>
> - for (j = 0; j < 2; j++) {
> + for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
> for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
> if (oqa[i].qt_dquot == NULL)
> break;
> @@ -138,8 +138,13 @@ xfs_trans_dup_dqinfo(
> oq->qt_ino_res = oq->qt_ino_res_used;
>
> }
> - oqa = otp->t_dqinfo->dqa_grpdquots;
> - nqa = ntp->t_dqinfo->dqa_grpdquots;
> + if (oqa == otp->t_dqinfo->dqa_usrdquots) {
> + oqa = otp->t_dqinfo->dqa_grpdquots;
> + nqa = ntp->t_dqinfo->dqa_grpdquots;
> + } else {
> + oqa = otp->t_dqinfo->dqa_prjdquots;
> + nqa = ntp->t_dqinfo->dqa_prjdquots;
> + }
> }
Ok, that's just plain nasty. And it's repeated nastiness.
I think that the best thing to do is redefine the struct
xfs_dquot_acct to something like:
enum {
XFS_QM_TRANS_USR = 0,
XFS_QM_TRANS_GRP,
XFS_QM_TRANS_PROJ,
XFS_QM_TRANS_DQTYPES,
};
#define XFS_QM_TRANS_MAXDQS 2
struct xfs_dquot_acct {
struct xfs_dqtrx dqs[XFS_QM_TRANS_DQTYPES][XFS_QM_TRANS_MAXDQS];
}
and that makes all these loops something like:
for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) {
oqa = &otp->t_dqinfo->dqs[j];
for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
....
}
}
And all this nastiness of selecting the next structure element goes
away.
> STATIC xfs_dqtrx_t *
> @@ -178,15 +185,20 @@ xfs_trans_get_dqtrx(
> int i;
> xfs_dqtrx_t *qa;
>
> - qa = XFS_QM_ISUDQ(dqp) ?
> - tp->t_dqinfo->dqa_usrdquots : tp->t_dqinfo->dqa_grpdquots;
> + if (XFS_QM_ISUDQ(dqp))
> + qa = tp->t_dqinfo->dqa_usrdquots;
qa = &tp->t_dqinfo->dqs[XFS_QM_TRANS_USR];
> + else if (XFS_QM_ISGDQ(dqp))
> + qa = tp->t_dqinfo->dqa_grpdquots;
qa = &tp->t_dqinfo->dqs[XFS_QM_TRANS_GRP];
> + else if (XFS_QM_ISPDQ(dqp))
> + qa = tp->t_dqinfo->dqa_prjdquots;
qa = &tp->t_dqinfo->dqs[XFS_QM_TRANS_PROJ];
> + else
> + return NULL;
>
> for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
> if (qa[i].qt_dquot == NULL ||
> qa[i].qt_dquot == dqp)
> return &qa[i];
> }
> -
> return NULL;
> }
>
> @@ -340,9 +352,12 @@ xfs_trans_apply_dquot_deltas(
>
> ASSERT(tp->t_dqinfo);
> qa = tp->t_dqinfo->dqa_usrdquots;
> - for (j = 0; j < 2; j++) {
> + for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
Comments aren't necessary like this if the above enum/array
construct is used.
> if (qa[0].qt_dquot == NULL) {
> - qa = tp->t_dqinfo->dqa_grpdquots;
> + if (qa == tp->t_dqinfo->dqa_usrdquots)
> + qa = tp->t_dqinfo->dqa_grpdquots;
> + else
> + qa = tp->t_dqinfo->dqa_prjdquots;
> continue;
> }
>
> @@ -496,9 +511,12 @@ xfs_trans_apply_dquot_deltas(
> be64_to_cpu(dqp->q_core.d_rtbcount));
> }
> /*
> - * Do the group quotas next
> + * Do the group quotas or project quotas next
> */
> - qa = tp->t_dqinfo->dqa_grpdquots;
> + if (qa == tp->t_dqinfo->dqa_usrdquots)
> + qa = tp->t_dqinfo->dqa_grpdquots;
> + else
> + qa = tp->t_dqinfo->dqa_prjdquots;
> }
> }
>
> @@ -523,7 +541,7 @@ xfs_trans_unreserve_and_mod_dquots(
>
> qa = tp->t_dqinfo->dqa_usrdquots;
>
> - for (j = 0; j < 2; j++) {
> + for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
> for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
> qtrx = &qa[i];
> /*
> @@ -565,7 +583,10 @@ xfs_trans_unreserve_and_mod_dquots(
> xfs_dqunlock(dqp);
>
> }
> - qa = tp->t_dqinfo->dqa_grpdquots;
> + if (qa == tp->t_dqinfo->dqa_usrdquots)
> + qa = tp->t_dqinfo->dqa_grpdquots;
> + else
> + qa = tp->t_dqinfo->dqa_prjdquots;
> }
And all this repeated nastiness goes away...
> }
>
> @@ -734,8 +755,8 @@ error_return:
>
> /*
> * Given dquot(s), make disk block and/or inode reservations against them.
> - * The fact that this does the reservation against both the usr and
> - * grp/prj quotas is important, because this follows a both-or-nothing
> + * The fact that this does the reservation against user, group and
> + * project quotas is important, because this follows a all-or-nothing
> * approach.
> *
> * flags = XFS_QMOPT_FORCE_RES evades limit enforcement. Used by chown.
> @@ -750,6 +771,7 @@ xfs_trans_reserve_quota_bydquots(
> xfs_mount_t *mp,
> xfs_dquot_t *udqp,
> xfs_dquot_t *gdqp,
> + xfs_dquot_t *pdqp,
> long nblks,
> long ninos,
> uint flags)
kill the typedefs.
> @@ -787,6 +809,24 @@ xfs_trans_reserve_quota_bydquots(
> }
> }
>
> + if (pdqp) {
> + error = xfs_trans_dqresv(tp, mp, pdqp, nblks, ninos, flags);
> + if (error) {
> + /*
> + * can't do it, so backout previous reservation
> + */
> + if (resvd) {
> + flags |= XFS_QMOPT_FORCE_RES;
> + xfs_trans_dqresv(tp, mp, udqp,
> + -nblks, -ninos, flags);
> + if (gdqp)
> + xfs_trans_dqresv(tp, mp, gdqp,
> + -nblks, -ninos, flags);
> + }
> + return error;
> + }
> + }
> +
This will only unwind group reservation is there was a user quota
reservation. I think that is wrong.
I think an unwind stack is better than the nested error
handling, and it removes the need for the "resvd" variable to
indicate that a user quota modification was made. i.e.
if (udqp) {
....
if (error)
return error;
....
}
if (gdqp) {
....
if (error)
goto unwind_usr;
....
}
if (pdqp) {
....
if (error)
goto unwind_grp;
....
}
unwind_grp:
flags |= XFS_QMOPT_FORCE_RES
if (gdqp)
xfs_trans_dqresv(tp, mp, gdqp, -nblks, -ninos, flags);
unwind_usr:
flags |= XFS_QMOPT_FORCE_RES
if (udqp)
xfs_trans_dqresv(tp, mp, udqp, -nblks, -ninos, flags);
return error;
> @@ -1498,7 +1502,7 @@ xfs_symlink(
> int n;
> xfs_buf_t *bp;
> prid_t prid;
> - struct xfs_dquot *udqp, *gdqp;
> + struct xfs_dquot *udqp = NULL, *gdqp = NULL, *pdqp = NULL;
One per line.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 3/5] xfs: Add pquotaino to on-disk super block
2012-07-20 23:02 ` [RFC v6 PATCH 3/5] xfs: Add pquotaino to on-disk super block Chandra Seetharaman
@ 2012-08-15 2:09 ` Dave Chinner
2012-08-22 23:30 ` Chandra Seetharaman
0 siblings, 1 reply; 19+ messages in thread
From: Dave Chinner @ 2012-08-15 2:09 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
On Fri, Jul 20, 2012 at 06:02:41PM -0500, Chandra Seetharaman wrote:
> Add a new field to the superblock to add support for seperate pquota
> with a specific version.
>
> Define a macro to differentiate superblock with and without OQUOTA.
>
> Keep backward compatibilty by alowing mount of older superblock
> with no separate pquota inode.
>
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> ---
> fs/xfs/xfs_itable.c | 3 +-
> fs/xfs/xfs_mount.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-
> fs/xfs/xfs_qm.c | 18 +++++++++----
> fs/xfs/xfs_qm_syscalls.c | 30 +++++++++++++++++-----
> fs/xfs/xfs_quota.h | 8 ------
> fs/xfs/xfs_sb.h | 20 ++++++++++++---
> fs/xfs/xfs_super.c | 15 +++++++----
> fs/xfs/xfs_trans_dquot.c | 4 ++-
> include/linux/dqblk_xfs.h | 1 +
> 9 files changed, 123 insertions(+), 34 deletions(-)
>
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index eff577a..0fa60b4 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -42,7 +42,8 @@ xfs_internal_inum(
> {
> return (ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> (xfs_sb_version_hasquota(&mp->m_sb) &&
> - (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino)));
> + (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino ||
> + ino == mp->m_sb.sb_pquotino)));
> }
>
> /*
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 22f23fd..992ec29 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -108,6 +108,7 @@ static const struct {
> { offsetof(xfs_sb_t, sb_logsunit), 0 },
> { offsetof(xfs_sb_t, sb_features2), 0 },
> { offsetof(xfs_sb_t, sb_bad_features2), 0 },
> + { offsetof(xfs_sb_t, sb_pquotino), 0 },
Can you line up the "0 }," part with the rest of the structure?
> { sizeof(xfs_sb_t), 0 }
> };
>
> @@ -618,6 +619,35 @@ 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);
> +
> + if (xfs_sb_version_has_no_oquota(to)) {
<crash>
My brain just core dumped on that name - "has no old quota"? "old
quota" could mean anything, and it doesn't describe the change in
on-disk format being made.
The feature being added is a "separate project quota inode", so the
feature bit and related functions need to reflect that. Hence I
think a better name is something like "has project quota inode" i.e
XFS_SB_VERSION2_PQUOTINO, xfs_sb_version_has_pquotino() and so on,
which matches the above addition to the superblock fields...
> + if (to->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> + xfs_notice(mp, "Super block has XFS_OQUOTA bits with "
> + "version NO_OQUOTA. Fixing it.\n");
> + to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> + }
> + to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
So we have a feature bit set, but old quota bits set. How can that
happen?
If it does occur, doesn't that mean we cannot trust the group or
project quotas to be correct, so at minimum this case needs to
trigger a quotacheck for both group and project quotas?
And if we can't enumerate how it can happen, then should we even
allow it to be "fixed" automatically?
> + } else {
> + if (to->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> + XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
> + xfs_notice(mp, "Super block has XFS_[G|P]UOTA bits in "
> + "older version. Fixing it.\n");
> + to->sb_qflags &= ~(XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> + XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD);
Again, how can that happen, and why isn't it a sign of corruption
that requires external intervention? If those bits are set, we can't
trust the quota to be correct, so at minimum we need to force a
quota check.
> + }
> + if (to->sb_qflags & XFS_OQUOTA_ENFD)
> + to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
> + XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
> + if (to->sb_qflags & XFS_OQUOTA_CHKD)
> + to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
> + XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
what do you do if both XFS_PQUOTA_ACCT and XFS_GQUOTA_ACCT are set?
i.e. both quotas were active even though the feature bit wasn't set?
This "fix it automatically" issue seems to be problematic to me
because it isn't clear whether we can get into these states, or if
we do, what the correct resolution of the problem is....
> + to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> +
> + if (to->sb_qflags & XFS_PQUOTA_ACCT) {
> + to->sb_pquotino = to->sb_gquotino;
> + to->sb_gquotino = NULLFSINO;
> + }
This needs a comment explaining that the in-memory code will use the
sb_pquotino for project quotas (i.e. behave as though the feature
bit is always set), so we ahve to set up the in-memory image that
way and convert it back to the old format when writing out the
superblock.
> + }
> }
>
> /*
> @@ -637,11 +667,22 @@ xfs_sb_to_disk(
> int first;
> int size;
> __uint16_t tmp16;
> + xfs_ino_t gquotino;
>
> ASSERT(fields);
> if (!fields)
> return;
>
> + /*
> + * On-disk version earlier than NO_OQUOTA doesn't have sb_pquotino.
> + * so, we need to copy the value to gquotino field.
> + */
> + if (!xfs_sb_version_has_no_oquota(from) &&
<boom>
Double negative! (not has no old quota).
Another reason for naming it PQUOTINO....
> + (from->sb_qflags & (XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD)))
> + gquotino = from->sb_pquotino;
> + else
> + gquotino = from->sb_gquotino;
Also, only do this transformation if we are modifying the group
quota inode.....
> +
> while (fields) {
> f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
> first = xfs_sb_info[f].offset;
> @@ -651,7 +692,8 @@ xfs_sb_to_disk(
>
> if (size == 1 || xfs_sb_info[f].type == 1) {
> memcpy(to_ptr + first, from_ptr + first, size);
> - } else if (f == XFS_SBS_QFLAGS) {
> + } else if ((f == XFS_SBS_QFLAGS) &&
[ no need for the extra (). ]
> + !xfs_sb_version_has_no_oquota(from)) {
> /*
> * The in-core version of sb_qflags do not have
> * XFS_OQUOTA_* flags, whereas the on-disk version
> @@ -671,6 +713,8 @@ xfs_sb_to_disk(
> tmp16 |= XFS_OQUOTA_CHKD;
>
> *(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
> + } else if (f == XFS_SBS_GQUOTINO) {
> + *(__be64 *)(to_ptr + first) = cpu_to_be64(gquotino);
.... i.e. here.
> } else {
> switch (size) {
> case 2:
> @@ -759,6 +803,12 @@ reread:
> goto reread;
> }
>
> + if (!xfs_sb_version_has_no_oquota(&mp->m_sb) &&
> + XFS_IS_PQUOTA_ON(mp)) {
> + mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
> + mp->m_sb.sb_gquotino = NULLFSINO;
> + }
why this is necessary? Didn't the earlier xfs_sb_from_disk() call
deal with this?
> +
> /* Initialize per-cpu counters */
> xfs_icsb_reinit_counters(mp);
>
> @@ -1646,6 +1696,12 @@ xfs_mod_sb(xfs_trans_t *tp, __int64_t fields)
> first = sizeof(xfs_sb_t);
> last = 0;
>
> + if (!xfs_sb_version_has_no_oquota(&mp->m_sb) &&
> + XFS_IS_PQUOTA_ON(mp)) {
> + fields &= (__int64_t)~XFS_SB_PQUOTINO;
> + fields |= (__int64_t)XFS_SB_GQUOTINO;
> + }
This will set the XFS_SB_GQUOTINO unconditionally. It only needs to
be set this if the XFS_SB_PQUOTINO field is set.
> @@ -838,19 +840,22 @@ xfs_qm_qino_alloc(
> ASSERT((sbfields & (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> XFS_SB_GQUOTINO | XFS_SB_QFLAGS)) ==
> (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> - XFS_SB_GQUOTINO | XFS_SB_QFLAGS));
> + XFS_SB_GQUOTINO | XFS_SB_PQUOTINO | XFS_SB_QFLAGS));
Did you test this with CONFIG_XFS_DEBUG=y? It will always fail
because you didn't add XFS_SB_PQUOTINO to the sbfields mask....
> @@ -1156,7 +1161,8 @@ xfs_qm_dqusage_adjust(
> * rootino must have its resources accounted for, not so with the quota
> * inodes.
> */
> - if (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino) {
> + if (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino ||
> + ino == mp->m_sb.sb_pquotino) {
if (ino == mp->m_sb.sb_uquotino ||
ino == mp->m_sb.sb_gquotino ||
ino == mp->m_sb.sb_pquotino) {
> @@ -413,17 +414,18 @@ xfs_qm_scall_getqstat(
> struct fs_quota_stat *out)
> {
> struct xfs_quotainfo *q = mp->m_quotainfo;
> - struct xfs_inode *uip, *gip;
> - boolean_t tempuqip, tempgqip;
> + struct xfs_inode *uip, *gip, *pip;
> + boolean_t tempuqip, tempgqip, temppqip;
>
> - uip = gip = NULL;
> - tempuqip = tempgqip = B_FALSE;
> + uip = gip = pip = NULL;
> + tempuqip = tempgqip = temppqip = B_FALSE;
Line per declaration and initialisation.
> diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
> index 8fd7894..7373108 100644
> --- a/fs/xfs/xfs_sb.h
> +++ b/fs/xfs/xfs_sb.h
> @@ -81,11 +81,15 @@ struct xfs_mount;
> #define XFS_SB_VERSION2_ATTR2BIT 0x00000008 /* Inline attr rework */
> #define XFS_SB_VERSION2_PARENTBIT 0x00000010 /* parent pointers */
> #define XFS_SB_VERSION2_PROJID32BIT 0x00000080 /* 32 bit project id */
> +#define XFS_SB_VERSION2_NO_OQUOTA 0x00000100 /* No OQUOTA and *
> + * separate project *
> + * quota field */
see my comments about naming this above.
> @@ -250,7 +255,7 @@ typedef enum {
> XFS_SBS_GQUOTINO, XFS_SBS_QFLAGS, XFS_SBS_FLAGS, XFS_SBS_SHARED_VN,
> XFS_SBS_INOALIGNMT, XFS_SBS_UNIT, XFS_SBS_WIDTH, XFS_SBS_DIRBLKLOG,
> XFS_SBS_LOGSECTLOG, XFS_SBS_LOGSECTSIZE, XFS_SBS_LOGSUNIT,
> - XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2,
> + XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2, XFS_SBS_PQUOTINO,
You got the name right here ;)
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 595f5ac..b475057 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -399,12 +399,6 @@ xfs_parseargs(
> }
> #endif
>
> - if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
> - (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE))) {
> - xfs_warn(mp, "cannot mount with both project and group quota");
> - return EINVAL;
> - }
> -
> if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
> xfs_warn(mp, "sunit and swidth must be specified together");
> return EINVAL;
> @@ -1330,6 +1324,15 @@ xfs_fs_fill_super(
> if (error)
> goto out_destroy_counters;
>
> + if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
> + (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE)) &&
> + !xfs_sb_version_has_no_oquota(&mp->m_sb)) {
> + xfs_warn(mp, "Super block does not support "
> + "project and group quota together");
> + error = EINVAL;
> + goto out_free_sb;
> + }
This check belongs in xfs_finish_flags().
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 4/5] xfs: Add proper versioning support to fs_quota_stat
2012-07-20 23:02 ` [RFC v6 PATCH 4/5] xfs: Add proper versioning support to fs_quota_stat Chandra Seetharaman
@ 2012-08-15 2:32 ` Dave Chinner
2012-08-22 23:32 ` Chandra Seetharaman
0 siblings, 1 reply; 19+ messages in thread
From: Dave Chinner @ 2012-08-15 2:32 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
On Fri, Jul 20, 2012 at 06:02:47PM -0500, Chandra Seetharaman wrote:
> Add proper versioning support for fs_quota_stat.
>
> Leave the earlier version as version 1 and add version 2 to add a new
> field "fs_qfilestat_t qs_pquota"
>
> Callers of the system call have to just set the version of the data
> structure being passed, and kernel will fill as much data as possible.
Userspace Interface Traps for the Unwary 101, below.
> /*
> * Disk quota - quotactl(2) commands for the XFS Quota Manager (XQM).
> @@ -139,6 +140,7 @@ typedef struct fs_disk_quota {
> * incore.
> */
> #define FS_QSTAT_VERSION 1 /* fs_quota_stat.qs_version */
> +#define FS_QSTAT_VERSION_2 2 /* new field qs_pquota */
>
> /*
> * Some basic information about 'quota files'.
> @@ -155,13 +157,37 @@ typedef struct fs_quota_stat {
> __s8 qs_pad; /* unused */
> fs_qfilestat_t qs_uquota; /* user quota storage information */
> fs_qfilestat_t qs_gquota; /* group quota storage information */
> -#define qs_pquota qs_gquota
> __u32 qs_incoredqs; /* number of dquots incore */
> __s32 qs_btimelimit; /* limit for blks timer */
> __s32 qs_itimelimit; /* limit for inodes timer */
> __s32 qs_rtbtimelimit;/* limit for rt blks timer */
> __u16 qs_bwarnlimit; /* limit for num warnings */
> __u16 qs_iwarnlimit; /* limit for num warnings */
> + fs_qfilestat_t qs_pquota; /* project quota storage information */
> } fs_quota_stat_t;
>
> +#define FS_QSTAT_V1_SIZE (offsetof(fs_quota_stat_t, qs_pquota))
> +#define FS_QSTAT_V2_SIZE (FS_QSTAT_V1_SIZE + sizeof (fs_qfilestat_t))
I don't expect that will work on all arches. pahole (everyone needs
to know about pahole!) tells me the original structure on x86_64
looks like:
struct fs_quota_stat {
__s8 qs_version; /* 0 1 */
/* XXX 1 byte hole, try to pack */
__u16 qs_flags; /* 2 2 */
__s8 qs_pad; /* 4 1 */
/* XXX 3 bytes hole, try to pack */
fs_qfilestat_t qs_uquota; /* 8 24 */
fs_qfilestat_t qs_gquota; /* 32 24 */
__u32 qs_incoredqs; /* 56 4 */
__s32 qs_btimelimit; /* 60 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
__s32 qs_itimelimit; /* 64 4 */
__s32 qs_rtbtimelimit; /* 68 4 */
__u16 qs_bwarnlimit; /* 72 2 */
__u16 qs_iwarnlimit; /* 74 2 */
/* size: 80, cachelines: 2, members: 11 */
/* sum members: 72, holes: 2, sum holes: 4 */
/* padding: 4 */
/* last cacheline: 16 bytes */
}
and that qs_iwarnlimit does not end on a 8 byte boundary. If we then
look at fs_qfilestat:
typedef struct fs_qfilestat {
__u64 qfs_ino; /* inode number */
__u64 qfs_nblks; /* number of BBs 512-byte-blks */
__u32 qfs_nextents; /* number of extents */
} fs_qfilestat_t;
it has an 8 byte alignment, so the above FS_QSTAT_V2_SIZE
calculation is wrong because it doesn't take into account holes in
the structure and there is 4 bytes of hole between qs_iwarnlimit and
qs_pquota. It's entirely possible that this might require a compat
handler as some platforms might pack the structure differently in 32
vs 64 bit binaries..
IOWs, you ned to explicitly add padding to thie structure, like
someone tried to do in the past a screwed it up completely.
Basically, the structure needs to be padded like this:
typedef struct fs_quota_stat {
__s8 qs_version; /* version number for future changes */
__u8 qs_pad1; /* unused */
__u16 qs_flags; /* FS_QUOTA_{U,P,G}DQ_{ACCT,ENFD} */
__u8 qs_pad2[3]; /* unused */
fs_qfilestat_t qs_uquota; /* user quota storage information */
fs_qfilestat_t qs_gquota; /* group quota storage information */
__u32 qs_incoredqs; /* number of dquots incore */
__s32 qs_btimelimit; /* limit for blks timer */
__s32 qs_itimelimit; /* limit for inodes timer */
__s32 qs_rtbtimelimit;/* limit for rt blks timer */
__u16 qs_bwarnlimit; /* limit for num warnings */
__u16 qs_iwarnlimit; /* limit for num warnings */
__u8 qs_pad3[4]; /* unused */
fs_qfilestat_t qs_pquota; /* project quota storage information */
} fs_quota_stat_t;
> +
> +static inline int valid_qstat_version(int version)
> +{
> + switch (version) {
> + case FS_QSTAT_VERSION:
> + case FS_QSTAT_VERSION_2:
> + return 1;
> + default:
> + return 0;
> + }
> +}
> +static inline int qstatsize(int version)
> +{
> + switch (version) {
> + case FS_QSTAT_VERSION_2:
> + return FS_QSTAT_V2_SIZE;
> + case FS_QSTAT_VERSION:
> + default:
> + return FS_QSTAT_V1_SIZE;
> + }
> +}
I don't see much point in these helpers - just put them in line in
the quota_getxstate() code. i.e.
switch (version) {
case FS_QSTAT_VERSION_2:
size = FS_QSTAT_V2_SIZE;
break;
default:
version = FS_QSTAT_VERSION;
/* fallthrough */
case FS_QSTAT_VERSION:
size = FS_QSTAT_V1_SIZE;
break;
}
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 5/5] xfs: Add a new field to fs_quota_stat to get pquota information
2012-07-20 23:03 ` [RFC v6 PATCH 5/5] xfs: Add a new field to fs_quota_stat to get pquota information Chandra Seetharaman
@ 2012-08-15 2:37 ` Dave Chinner
2012-08-22 23:40 ` Chandra Seetharaman
0 siblings, 1 reply; 19+ messages in thread
From: Dave Chinner @ 2012-08-15 2:37 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
On Fri, Jul 20, 2012 at 06:03:05PM -0500, Chandra Seetharaman wrote:
> Use separate structure for filling the project quota information
> for the Q_XGETQSTAT quota command.
>
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> ---
> fs/xfs/xfs_qm_syscalls.c | 26 +++++++++++++-------------
> 1 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index f011cf3..482efc0 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -421,7 +421,6 @@ xfs_qm_scall_getqstat(
> tempuqip = tempgqip = temppqip = B_FALSE;
> memset(out, 0, sizeof(fs_quota_stat_t));
>
> - out->qs_version = FS_QSTAT_VERSION;
> if (!xfs_sb_version_hasquota(&mp->m_sb)) {
> out->qs_uquota.qfs_ino = NULLFSINO;
> out->qs_gquota.qfs_ino = NULLFSINO;
> @@ -434,8 +433,6 @@ xfs_qm_scall_getqstat(
> out->qs_pad = 0;
> out->qs_uquota.qfs_ino = mp->m_sb.sb_uquotino;
> out->qs_gquota.qfs_ino = mp->m_sb.sb_gquotino;
> - if (&out->qs_gquota != &out->qs_pquota)
> - out->qs_pquota.qfs_ino = mp->m_sb.sb_pquotino;
>
> if (q) {
> uip = q->qi_uquotaip;
> @@ -452,11 +449,6 @@ xfs_qm_scall_getqstat(
> 0, 0, &gip) == 0)
> tempgqip = B_TRUE;
> }
> - if (!pip && mp->m_sb.sb_pquotino != NULLFSINO) {
> - if (xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
> - 0, 0, &pip) == 0)
> - temppqip = B_TRUE;
> - }
> if (uip) {
> out->qs_uquota.qfs_nblks = uip->i_d.di_nblocks;
> out->qs_uquota.qfs_nextents = uip->i_d.di_nextents;
> @@ -469,11 +461,19 @@ xfs_qm_scall_getqstat(
> if (tempgqip)
> IRELE(gip);
> }
> - if (pip) {
> - out->qs_pquota.qfs_nblks = pip->i_d.di_nblocks;
> - out->qs_pquota.qfs_nextents = pip->i_d.di_nextents;
> - if (temppqip)
> - IRELE(pip);
> + if (out->qs_version >= FS_QSTAT_VERSION_2) {
> + out->qs_pquota.qfs_ino = mp->m_sb.sb_pquotino;
> + if (!pip && mp->m_sb.sb_pquotino != NULLFSINO) {
> + if (xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
> + 0, 0, &pip) == 0)
> + temppqip = B_TRUE;
> + }
> + if (pip) {
> + out->qs_pquota.qfs_nblks = pip->i_d.di_nblocks;
> + out->qs_pquota.qfs_nextents = pip->i_d.di_nextents;
> + if (temppqip)
> + IRELE(pip);
> + }
Doesn't this mean that we'll never report project quota for
FS_QSTAT_VERSION structures and the old quota format? i.e. existing
tools with a new kernel won't return the project quota info in the
group quota fields?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD
2012-08-14 22:46 ` Dave Chinner
@ 2012-08-22 22:53 ` Chandra Seetharaman
2012-09-13 7:56 ` Christoph Hellwig
0 siblings, 1 reply; 19+ messages in thread
From: Chandra Seetharaman @ 2012-08-22 22:53 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
will fix them as suggested.
Thanks for the review.
Chandra
On Wed, 2012-08-15 at 08:46 +1000, Dave Chinner wrote:
> On Fri, Jul 20, 2012 at 06:02:08PM -0500, Chandra Seetharaman wrote:
> > Remove all incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD. Instead,
> > start using XFS_GQUOTA_.* XFS_PQUOTA_.* counterparts for GQUOTA nd
> > PQUOTA respectively.
> >
> > No changes are made to the on-disk version of the superblock yet. On-disk
> > copy still uses XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD.
> >
> > Read and write of the superblock does the conversion from *OQUOTA*
> > to *[PG]QUOTA*.
>
> Couple of minor style things....
>
> > @@ -622,6 +636,7 @@ xfs_sb_to_disk(
> > xfs_sb_field_t f;
> > int first;
> > int size;
> > + __uint16_t tmp16;
>
> tmp16 is a bad name for a temporary variable. Move it to the scope
> that uses it, and name it for it's purpose. i.e:
>
>
> >
> > ASSERT(fields);
> > if (!fields)
> > @@ -636,6 +651,26 @@ xfs_sb_to_disk(
> >
> > if (size == 1 || xfs_sb_info[f].type == 1) {
> > memcpy(to_ptr + first, from_ptr + first, size);
> > + } else if (f == XFS_SBS_QFLAGS) {
>
> __uint16_t qflags;
>
> > + /*
> > + * The in-core version of sb_qflags do not have
> > + * XFS_OQUOTA_* flags, whereas the on-disk version
> > + * does. Save the in-core sb_qflags temporarily,
> > + * removing the new XFS_{PG}QUOTA_* flags and re-apply
> > + * the old on-disk flags.
> > + */
> > + tmp16 = from->sb_qflags &
>
> qflags = from->sb_qflags & ....
>
> > + ~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
> > + XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
> > +
> > + if (from->sb_qflags &
> > + (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
> > + tmp16 |= XFS_OQUOTA_ENFD;
> > + if (from->sb_qflags &
> > + (XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
> > + tmp16 |= XFS_OQUOTA_CHKD;
> > +
> > + *(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
> > } else {
> > switch (size) {
> > case 2:
>
> ....
>
> > @@ -339,9 +339,11 @@ xfs_qm_scall_quotaon(
> > ||
> > ((flags & XFS_PQUOTA_ACCT) == 0 &&
> > (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT) == 0 &&
> > - (flags & XFS_GQUOTA_ACCT) == 0 &&
> > + (flags & XFS_PQUOTA_ENFD))
> > + ||
> > + ((flags & XFS_GQUOTA_ACCT) == 0 &&
> > (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT) == 0 &&
> > - (flags & XFS_OQUOTA_ENFD))) {
> > + (flags & XFS_GQUOTA_ENFD))) {
>
> Can you fix the flat indenting here at the same time so that the
> logic is obvious at a glance and consistent with the rest of the XFS
> code? i.e.
>
> if (((flags & XFS_UQUOTA_ACCT) == 0 &&
> (mp->m_sb.sb_qflags & XFS_UQUOTA_ACCT) == 0 &&
> (flags & XFS_UQUOTA_ENFD)) ||
> ((flags & XFS_PQUOTA_ACCT) == 0 &&
> (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT) == 0 &&
> (flags & XFS_PQUOTA_ENFD)) ||
> (flags & XFS_GQUOTA_ACCT) == 0 &&
> (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT) == 0 &&
> (flags & XFS_GQUOTA_ENFD))) {
>
> > xfs_debug(mp,
> > "%s: Can't enforce without acct, flags=%x sbflags=%x\n",
> > __func__, flags, mp->m_sb.sb_qflags);
> > @@ -771,8 +773,10 @@ xfs_qm_scall_getquota(
> > * so return zeroes in that case.
> > */
> > if ((!XFS_IS_UQUOTA_ENFORCED(mp) && dqp->q_core.d_flags == XFS_DQ_USER) ||
> > - (!XFS_IS_OQUOTA_ENFORCED(mp) &&
> > - (dqp->q_core.d_flags & (XFS_DQ_PROJ | XFS_DQ_GROUP)))) {
> > + (!XFS_IS_PQUOTA_ENFORCED(mp)
> > + && dqp->q_core.d_flags == XFS_DQ_PROJ) ||
> > + (!XFS_IS_GQUOTA_ENFORCED(mp)
> > + && dqp->q_core.d_flags == XFS_DQ_GROUP)) {
>
> Same here:
>
> if ((!XFS_IS_UQUOTA_ENFORCED(mp) &&
> dqp->q_core.d_flags == XFS_DQ_USER) ||
> (!XFS_IS_PQUOTA_ENFORCED(mp) &&
> dqp->q_core.d_flags == XFS_DQ_PROJ) ||
> (!XFS_IS_GQUOTA_ENFORCED(mp) &&
> dqp->q_core.d_flags == XFS_DQ_GROUP)) {
>
> Otherwise it looks good. Fix the above and you can add a
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 2/5] xfs: Add pquota fields where gquota is used.
2012-08-15 1:15 ` Dave Chinner
@ 2012-08-22 22:56 ` Chandra Seetharaman
0 siblings, 0 replies; 19+ messages in thread
From: Chandra Seetharaman @ 2012-08-22 22:56 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
Will rework as per your suggestion.
Thanks for the review.
Chandra
On Wed, 2012-08-15 at 11:15 +1000, Dave Chinner wrote:
> On Fri, Jul 20, 2012 at 06:02:25PM -0500, Chandra Seetharaman wrote:
> > Add project quota changes to all the places where group quota field
> > is used:
> > * add separate project quota members into various structures
> > * split project quota and group quotas so that instead of overriding
> > the group quota members incore, the new project quota members are
> > used instead
> > * get rid of usage of the OQUOTA flag incore, in favor of separate group
> > and project quota flags.
> > * add a project dquot argument to various functions.
> >
> > No externally visible interfaces changed and no superblock changes yet.
>
> Lots of comments below.
>
> >
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > ---
> > fs/xfs/xfs_dquot.c | 16 ++-
> > fs/xfs/xfs_dquot.h | 11 ++-
> > fs/xfs/xfs_iget.c | 2 +-
> > fs/xfs/xfs_inode.h | 1 +
> > fs/xfs/xfs_ioctl.c | 14 ++--
> > fs/xfs/xfs_iops.c | 4 +-
> > fs/xfs/xfs_qm.c | 255 ++++++++++++++++++++++++++++++++--------------
> > fs/xfs/xfs_qm.h | 15 ++--
> > fs/xfs/xfs_qm_bhv.c | 2 +-
> > fs/xfs/xfs_qm_syscalls.c | 19 +++-
> > fs/xfs/xfs_quota.h | 38 ++++---
> > fs/xfs/xfs_sb.h | 1 +
> > fs/xfs/xfs_super.c | 5 +-
> > fs/xfs/xfs_trans_dquot.c | 71 ++++++++++---
> > fs/xfs/xfs_vnodeops.c | 23 +++--
> > 15 files changed, 326 insertions(+), 151 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index bf27fcc..42b8b79 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -751,7 +751,7 @@ xfs_qm_dqput_final(
> > struct xfs_dquot *dqp)
> > {
> > struct xfs_quotainfo *qi = dqp->q_mount->m_quotainfo;
> > - struct xfs_dquot *gdqp;
> > + struct xfs_dquot *gdqp, *pdqp;
>
> We tend not to declare multiple variables on the one line - just add
> a new line with the new variable.
>
> >
> > trace_xfs_dqput_free(dqp);
> >
> > @@ -765,21 +765,29 @@ xfs_qm_dqput_final(
> >
> > /*
> > * If we just added a udquot to the freelist, then we want to release
> > - * the gdquot reference that it (probably) has. Otherwise it'll keep
> > - * the gdquot from getting reclaimed.
> > + * the gdquot/pdquot reference that it (probably) has. Otherwise it'll
> > + * keep the gdquot/pdquot from getting reclaimed.
> > */
> > gdqp = dqp->q_gdquot;
> > if (gdqp) {
> > xfs_dqlock(gdqp);
> > dqp->q_gdquot = NULL;
> > }
> > +
> > + pdqp = dqp->q_pdquot;
> > + if (pdqp) {
> > + xfs_dqlock(pdqp);
> > + dqp->q_pdquot = NULL;
> > + }
>
> seeing this (and the various dupilications in this patch) makes me
> wonder if we'd be better off with array based abstractions and
> loops. That's not important for this patch set, but if we ever want
> to add another type of quota, then it would make sense...
>
> > diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
> > index 7d20af2..893cd5e 100644
> > --- a/fs/xfs/xfs_dquot.h
> > +++ b/fs/xfs/xfs_dquot.h
> > @@ -46,6 +46,7 @@ typedef struct xfs_dquot {
> > xfs_fileoff_t q_fileoffset; /* offset in quotas file */
> >
> > struct xfs_dquot*q_gdquot; /* group dquot, hint only */
> > + struct xfs_dquot *q_pdquot; /* project dquot, hint only */
>
> You may as well put a space in the q_gdquot declaration so they are
> consistent....
>
> > + return ip->i_pdquot;
> > default:
> > return NULL;
> > }
> > @@ -136,7 +139,9 @@ static inline xfs_dquot_t *xfs_inode_dquot(struct xfs_inode *ip, int type)
> > #define XFS_DQ_TO_QINF(dqp) ((dqp)->q_mount->m_quotainfo)
> > #define XFS_DQ_TO_QIP(dqp) (XFS_QM_ISUDQ(dqp) ? \
> > XFS_DQ_TO_QINF(dqp)->qi_uquotaip : \
> > - XFS_DQ_TO_QINF(dqp)->qi_gquotaip)
> > + (XFS_QM_ISGDQ(dqp) ? \
> > + XFS_DQ_TO_QINF(dqp)->qi_gquotaip : \
> > + XFS_DQ_TO_QINF(dqp)->qi_pquotaip))
>
> nested ?: constructs are a bit nasty. Perhaps this should be
> converted to a static inline function like:
>
> static inline struct xfs_inode *
> xfs_dq_to_quota_inode(
> struct xfs_dquot *dqp)
> {
> if (XFS_QM_ISUDQ(dqp))
> return dqp->q_mount->m_quotainfo->qi_uquotaip;
> if (XFS_QM_ISGDQ(dqp))
> return dqp->q_mount->m_quotainfo->qi_gquotaip;
> ASSERT(XFS_QM_ISPDQ(dqp)));
> return dqp->q_mount->m_quotainfo->qi_pquotaip;
> }
>
>
> >
> > extern int xfs_qm_dqread(struct xfs_mount *, xfs_dqid_t, uint,
> > uint, struct xfs_dquot **);
> > diff --git a/fs/xfs/xfs_iget.c b/fs/xfs/xfs_iget.c
> > index 1bb4365..e97fb18 100644
> > --- a/fs/xfs/xfs_iget.c
> > +++ b/fs/xfs/xfs_iget.c
> > @@ -346,7 +346,7 @@ xfs_iget_cache_miss(
> > iflags = XFS_INEW;
> > if (flags & XFS_IGET_DONTCACHE)
> > iflags |= XFS_IDONTCACHE;
> > - ip->i_udquot = ip->i_gdquot = NULL;
> > + ip->i_udquot = ip->i_gdquot = ip->i_pdquot = NULL;
>
> That's getting a little messy. I think you should convert these to
> an assignment per line. i.e:
>
> ip->i_udquot = NULL;
> ip->i_gdquot = NULL;
> ip->i_pdquot = NULL;
>
> > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > index 3fb3730..46c7e4c 100644
> > --- a/fs/xfs/xfs_qm.c
> > +++ b/fs/xfs/xfs_qm.c
> > @@ -134,7 +134,7 @@ xfs_qm_dqpurge(
> > {
> > struct xfs_mount *mp = dqp->q_mount;
> > struct xfs_quotainfo *qi = mp->m_quotainfo;
> > - struct xfs_dquot *gdqp = NULL;
> > + struct xfs_dquot *gdqp = NULL, *pdqp = NULL;
>
> New variable on a new line.
>
> >
> > xfs_dqlock(dqp);
> > if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) {
> > @@ -143,8 +143,8 @@ xfs_qm_dqpurge(
> > }
> >
> > /*
> > - * If this quota has a group hint attached, prepare for releasing it
> > - * now.
> > + * If this quota has a group/project hint attached, prepare for
> > + * releasing it now.
>
> I'd just say "If this quota has a hint attached..."
>
> > /*
> > - * Given a udquot and gdquot, attach a ptr to the group dquot in the
> > + * Given a udquot and gdquot, attach a ptr to the group/project dquot in the
> > * udquot as a hint for future lookups.
> > */
> > STATIC void
> > -xfs_qm_dqattach_grouphint(
> > - xfs_dquot_t *udq,
> > - xfs_dquot_t *gdq)
> > +xfs_qm_dqattach_grouphint(xfs_inode_t *ip, int type)
> > {
> > - xfs_dquot_t *tmp;
> > + xfs_dquot_t **tmp, *gpdq, *tmp1, *udq = ip->i_udquot;
>
> Don't use typedefs for new definitions, tmp is a really bad name,
> and use consistent style:
>
> STATIC void
> xfs_qm_dqattach_grouphint(
> struct xfs_inode *ip,
> int type)
> {
> struct xfs_dquot *udq = ip->i_udquot;
> struct xfs_dquot *gpdq;
> struct xfs_dquot **dqhint;
> struct xfs_dquot *tmp1;
>
> > + gpdq = (type == XFS_DQ_GROUP) ? ip->i_gdquot : ip->i_pdquot;
> > xfs_dqlock(udq);
> >
> > - tmp = udq->q_gdquot;
> > - if (tmp) {
> > - if (tmp == gdq)
> > + tmp = (type == XFS_DQ_GROUP) ? &udq->q_gdquot : &udq->q_pdquot;
>
> if (type == XFS_DQ_GROUP) {
> gpdq = ip->i_gdquot;
> dqhint = &udq->q_gdquot;
> } else {
> gpdq = ip->i_gdquot;
> dqhint = &udq->q_gdquot;
> }
>
> > +
> > + if (*tmp) {
> > + if (*tmp == gpdq)
> > goto done;
> >
> > - udq->q_gdquot = NULL;
> > - xfs_qm_dqrele(tmp);
> > + tmp1 = *tmp;
> > + *tmp = NULL;
> > + xfs_qm_dqrele(tmp1);
> > }
> >
> > - udq->q_gdquot = xfs_qm_dqhold(gdq);
> > + *tmp = xfs_qm_dqhold(gpdq);
>
> if (*dqhint) {
> struct xfs_dquot *tmp;
>
> if (*dqhint == gpdq)
> goto done;
>
> tmp = *dqhint;
> *dqhint = NULL;
> xfs_qm_rele(tmp);
> }
> *dqhint = xfs_qm_dqhold(gpdq);
>
> And when we get rid of the tmp names, all of a sudden the code goes
> from being unreadable to being obviously suboptimal. i.e:
>
> if (*dqhint == gpdq)
> goto done;
>
> if (*dqhint)
> xfs_qm_rele(*dqhint);
> *dqhint = xfs_qm_dqhold(gpdq);
>
> We don't need the second temp variable because we have the dquot
> locked and so nobody is going to be accessing the hint in the dquot
> after we've released it. If they are accessing it unlocked, then it
> is already racy and setting the dqhint to null doesn't change
> anything....
>
> > @@ -1227,7 +1269,7 @@ xfs_qm_quotacheck(
> > int done, count, error, error2;
> > xfs_ino_t lastino;
> > size_t structsz;
> > - xfs_inode_t *uip, *gip;
> > + xfs_inode_t *uip, *gip, *pip;
>
> struct xfs_inode *uip = mp->m_quotainfo->qi_uquotaip;
> struct xfs_inode *gip = mp->m_quotainfo->qi_gquotaip;
> struct xfs_inode *pip = mp->m_quotainfo->qi_pquotaip;
>
> > uint flags;
> > LIST_HEAD (buffer_list);
> >
> > @@ -1236,7 +1278,8 @@ xfs_qm_quotacheck(
> > lastino = 0;
> > flags = 0;
> >
> > - ASSERT(mp->m_quotainfo->qi_uquotaip || mp->m_quotainfo->qi_gquotaip);
> > + ASSERT(mp->m_quotainfo->qi_uquotaip || mp->m_quotainfo->qi_gquotaip
> > + || mp->m_quotainfo->qi_pquotaip);
>
> ASSERT(uip || gip || pip);
>
> > ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> >
> > xfs_notice(mp, "Quotacheck needed: Please wait.");
> > @@ -1257,13 +1300,20 @@ xfs_qm_quotacheck(
> >
> > gip = mp->m_quotainfo->qi_gquotaip;
> > if (gip) {
> > - error = xfs_qm_dqiterate(mp, gip, XFS_IS_GQUOTA_ON(mp) ?
> > - XFS_QMOPT_GQUOTA : XFS_QMOPT_PQUOTA,
> > + error = xfs_qm_dqiterate(mp, gip, XFS_QMOPT_GQUOTA,
> > &buffer_list);
> > if (error)
> > goto error_return;
> > - flags |= XFS_IS_GQUOTA_ON(mp) ?
> > - XFS_GQUOTA_CHKD : XFS_PQUOTA_CHKD;
> > + flags |= XFS_GQUOTA_CHKD;
> > + }
> > +
> > + pip = mp->m_quotainfo->qi_pquotaip;
> > + if (pip) {
> > + error = xfs_qm_dqiterate(mp, pip, XFS_QMOPT_PQUOTA,
> > + &buffer_list);
> > + if (error)
> > + goto error_return;
> > + flags |= XFS_PQUOTA_CHKD;
> > }
> >
> > do {
> > @@ -1358,13 +1408,13 @@ STATIC int
> > xfs_qm_init_quotainos(
> > xfs_mount_t *mp)
> > {
> > - xfs_inode_t *uip, *gip;
> > + xfs_inode_t *uip, *gip, *pip;
> > int error;
> > __int64_t sbflags;
> > uint flags;
> >
> > ASSERT(mp->m_quotainfo);
> > - uip = gip = NULL;
> > + uip = gip = pip = NULL;
>
> Same as above.
>
> > sbflags = 0;
> > flags = 0;
> >
> > @@ -1379,7 +1429,7 @@ xfs_qm_init_quotainos(
> > 0, 0, &uip)))
> > return XFS_ERROR(error);
> > }
> > - if (XFS_IS_OQUOTA_ON(mp) &&
> > + if (XFS_IS_GQUOTA_ON(mp) &&
> > mp->m_sb.sb_gquotino != NULLFSINO) {
> > ASSERT(mp->m_sb.sb_gquotino > 0);
> > if ((error = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino,
> > @@ -1389,6 +1439,19 @@ xfs_qm_init_quotainos(
> > return XFS_ERROR(error);
> > }
> > }
> > + if (XFS_IS_PQUOTA_ON(mp) &&
> > + mp->m_sb.sb_pquotino != NULLFSINO) {
> > + ASSERT(mp->m_sb.sb_pquotino > 0);
> > + error = xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
> > + 0, 0, &pip);
> > + if (error) {
> > + if (uip)
> > + IRELE(uip);
> > + if (gip)
> > + IRELE(gip);
> > + return XFS_ERROR(error);
> > + }
>
> This error handling is repeated a couple of times. It is better to
> use an error stack at the end of the function for this. i.e.
>
> if (error)
> goto error_rele;
> ...
>
> return 0;
>
> error_rele:
> if (uip)
> IRELE(uip);
> if (gip)
> IRELE(gip);
> if (pip)
> IRELE(pip);
> return XFS_ERROR(error);
> }
>
> > @@ -1621,10 +1697,11 @@ xfs_qm_vop_dqalloc(
> > prid_t prid,
> > uint flags,
> > struct xfs_dquot **O_udqpp,
> > - struct xfs_dquot **O_gdqpp)
> > + struct xfs_dquot **O_gdqpp,
> > + struct xfs_dquot **O_pdqpp)
> > {
> > struct xfs_mount *mp = ip->i_mount;
> > - struct xfs_dquot *uq, *gq;
> > + struct xfs_dquot *uq, *gq, *pq;
>
> Separate lines with initialisation...
>
> > int error;
> > uint lockflags;
> >
> > @@ -1649,7 +1726,7 @@ xfs_qm_vop_dqalloc(
> > }
> > }
> >
> > - uq = gq = NULL;
> > + uq = gq = pq = NULL;
>
> So this can be killed.
>
> > if ((flags & XFS_QMOPT_UQUOTA) && XFS_IS_UQUOTA_ON(mp)) {
> > if (ip->i_d.di_uid != uid) {
> > /*
> > @@ -1705,25 +1782,28 @@ xfs_qm_vop_dqalloc(
> > ASSERT(ip->i_gdquot);
> > gq = xfs_qm_dqhold(ip->i_gdquot);
> > }
> > - } else if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
> > + }
> > + if ((flags & XFS_QMOPT_PQUOTA) && XFS_IS_PQUOTA_ON(mp)) {
> > if (xfs_get_projid(ip) != prid) {
> > xfs_iunlock(ip, lockflags);
> > if ((error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)prid,
> > XFS_DQ_PROJ,
> > XFS_QMOPT_DQALLOC |
> > XFS_QMOPT_DOWARN,
> > - &gq))) {
> > + &pq))) {
>
> Separate the function call fro the if() statement. Also, both dqid_t
> and prid_t are uint32_t, so there is no need for a cast:
>
> error = xfs_qm_dqget(mp, NULL, prid, XFS_DQ_PROJ,
> XFS_QMOPT_DQALLOC | XFS_QMOPT_DOWARN,
> &gq);
> if (error) {
> ....
>
> > @@ -1790,11 +1874,13 @@ xfs_qm_vop_chown_reserve(
> > xfs_inode_t *ip,
> > xfs_dquot_t *udqp,
> > xfs_dquot_t *gdqp,
> > + xfs_dquot_t *pdqp,
>
> Probably should remove the typedefs while adding new parameters.
> > uint flags)
> > {
> > xfs_mount_t *mp = ip->i_mount;
> > uint delblks, blkflags, prjflags = 0;
> > - xfs_dquot_t *unresudq, *unresgdq, *delblksudq, *delblksgdq;
> > + xfs_dquot_t *unresudq, *unresgdq, *unrespdq;
> > + xfs_dquot_t *delblksudq, *delblksgdq, *delblkspdq;
> > int error;
>
> Same here, and use one variable per line with initialisation.
> >
> >
> > @@ -1802,7 +1888,8 @@ xfs_qm_vop_chown_reserve(
> > ASSERT(XFS_IS_QUOTA_RUNNING(mp));
> >
> > delblks = ip->i_delayed_blks;
> > - delblksudq = delblksgdq = unresudq = unresgdq = NULL;
> > + delblksudq = delblksgdq = delblkspdq = NULL;
> > + unresudq = unresgdq = unrespdq = NULL;
>
> So this can be removed.
>
> > blkflags = XFS_IS_REALTIME_INODE(ip) ?
> > XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS;
> >
> > @@ -1819,25 +1906,28 @@ xfs_qm_vop_chown_reserve(
> > unresudq = ip->i_udquot;
> > }
> > }
> > - if (XFS_IS_OQUOTA_ON(ip->i_mount) && gdqp) {
> > - if (XFS_IS_PQUOTA_ON(ip->i_mount) &&
> > - xfs_get_projid(ip) != be32_to_cpu(gdqp->q_core.d_id))
> > - prjflags = XFS_QMOPT_ENOSPC;
> > -
> > - if (prjflags ||
> > - (XFS_IS_GQUOTA_ON(ip->i_mount) &&
> > - ip->i_d.di_gid != be32_to_cpu(gdqp->q_core.d_id))) {
> > - delblksgdq = gdqp;
> > - if (delblks) {
> > - ASSERT(ip->i_gdquot);
> > - unresgdq = ip->i_gdquot;
> > - }
> > + if (XFS_IS_GQUOTA_ON(ip->i_mount) && gdqp &&
> > + ip->i_d.di_gid != be32_to_cpu(gdqp->q_core.d_id)) {
> > + delblksgdq = gdqp;
> > + if (delblks) {
> > + ASSERT(ip->i_gdquot);
> > + unresgdq = ip->i_gdquot;
> > + }
> > + }
> > +
> > + if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
> > + xfs_get_projid(ip) != be32_to_cpu(pdqp->q_core.d_id)) {
> > + prjflags = XFS_QMOPT_ENOSPC;
> > + delblkspdq = pdqp;
> > + if (delblks) {
> > + ASSERT(ip->i_pdquot);
> > + unrespdq = ip->i_pdquot;
> > }
> > }
> >
> > if ((error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
> > - delblksudq, delblksgdq, ip->i_d.di_nblocks, 1,
> > - flags | blkflags | prjflags)))
> > + delblksudq, delblksgdq, delblkspdq, ip->i_d.di_nblocks,
> > + 1, flags | blkflags | prjflags)))
>
> Separate the function call from the if().
>
> > return (error);
> >
> > /*
> > @@ -1850,15 +1940,16 @@ xfs_qm_vop_chown_reserve(
> > /*
> > * Do the reservations first. Unreservation can't fail.
> > */
> > - ASSERT(delblksudq || delblksgdq);
> > - ASSERT(unresudq || unresgdq);
> > + ASSERT(delblksudq || delblksgdq || delblkspdq);
> > + ASSERT(unresudq || unresgdq || unrespdq);
> > if ((error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
> > - delblksudq, delblksgdq, (xfs_qcnt_t)delblks, 0,
> > + delblksudq, delblksgdq, delblkspdq,
> > + (xfs_qcnt_t)delblks, 0,
> > flags | blkflags | prjflags)))
>
> Same again.
>
> > diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> > index 44b858b..256ff71 100644
> > --- a/fs/xfs/xfs_qm.h
> > +++ b/fs/xfs/xfs_qm.h
> > @@ -44,9 +44,11 @@ extern struct kmem_zone *xfs_qm_dqtrxzone;
> > typedef struct xfs_quotainfo {
> > struct radix_tree_root qi_uquota_tree;
> > struct radix_tree_root qi_gquota_tree;
> > + struct radix_tree_root qi_pquota_tree;
> > struct mutex qi_tree_lock;
> > xfs_inode_t *qi_uquotaip; /* user quota inode */
> > xfs_inode_t *qi_gquotaip; /* group quota inode */
> > + xfs_inode_t *qi_pquotaip; /* project quota inode */
>
> convert to struct xfs_inode.
>
> > struct list_head qi_lru_list;
> > struct mutex qi_lru_lock;
> > int qi_lru_count;
> > @@ -70,26 +72,25 @@ typedef struct xfs_quotainfo {
> > } xfs_quotainfo_t;
> >
> > #define XFS_DQUOT_TREE(qi, type) \
> > - ((type & XFS_DQ_USER) ? \
> > - &((qi)->qi_uquota_tree) : \
> > - &((qi)->qi_gquota_tree))
> > + ((type & XFS_DQ_USER) ? &((qi)->qi_uquota_tree) : \
> > + ((type & XFS_DQ_GROUP) ? &((qi)->qi_gquota_tree) : \
> > + &((qi)->qi_pquota_tree)))
>
> Convert to static inline, I think.
>
> static inline struct radix_tree_root *
> xfs_dquot_tree(
> struct xfs_quotainfo *qi,
> int type)
> {
> switch(type) {
> case XFS_DQ_USER:
> return qi->qi_uquota_tree;
> case XFS_DQ_GROUP:
> return qi->qi_gquota_tree;
> case XFS_DQ_PROJ:
> return qi->qi_pquota_tree;
> default:
> ASSERT(0);
> }
> return NULL;
> }
>
> > @@ -267,8 +265,10 @@ typedef struct xfs_qoff_logformat {
> > */
> > #define XFS_NOT_DQATTACHED(mp, ip) ((XFS_IS_UQUOTA_ON(mp) &&\
> > (ip)->i_udquot == NULL) || \
> > - (XFS_IS_OQUOTA_ON(mp) && \
> > - (ip)->i_gdquot == NULL))
> > + (XFS_IS_GQUOTA_ON(mp) && \
> > + (ip)->i_gdquot == NULL) || \
> > + (XFS_IS_PQUOTA_ON(mp) && \
> > + (ip)->i_pdquot == NULL))
>
> #define XFS_NOT_DQATTACHED(mp, ip) \
> ((XFS_IS_UQUOTA_ON(mp) && (ip)->i_udquot == NULL) || \
> (XFS_IS_GQUOTA_ON(mp) && (ip)->i_gdquot == NULL) || \
> (XFS_IS_PQUOTA_ON(mp) && (ip)->i_pdquot == NULL))
>
> > +++ b/fs/xfs/xfs_trans_dquot.c
> > @@ -113,7 +113,7 @@ xfs_trans_dup_dqinfo(
> > if(otp->t_flags & XFS_TRANS_DQ_DIRTY)
> > ntp->t_flags |= XFS_TRANS_DQ_DIRTY;
> >
> > - for (j = 0; j < 2; j++) {
> > + for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
> > for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
> > if (oqa[i].qt_dquot == NULL)
> > break;
> > @@ -138,8 +138,13 @@ xfs_trans_dup_dqinfo(
> > oq->qt_ino_res = oq->qt_ino_res_used;
> >
> > }
> > - oqa = otp->t_dqinfo->dqa_grpdquots;
> > - nqa = ntp->t_dqinfo->dqa_grpdquots;
> > + if (oqa == otp->t_dqinfo->dqa_usrdquots) {
> > + oqa = otp->t_dqinfo->dqa_grpdquots;
> > + nqa = ntp->t_dqinfo->dqa_grpdquots;
> > + } else {
> > + oqa = otp->t_dqinfo->dqa_prjdquots;
> > + nqa = ntp->t_dqinfo->dqa_prjdquots;
> > + }
> > }
>
> Ok, that's just plain nasty. And it's repeated nastiness.
>
> I think that the best thing to do is redefine the struct
> xfs_dquot_acct to something like:
>
> enum {
> XFS_QM_TRANS_USR = 0,
> XFS_QM_TRANS_GRP,
> XFS_QM_TRANS_PROJ,
> XFS_QM_TRANS_DQTYPES,
> };
> #define XFS_QM_TRANS_MAXDQS 2
> struct xfs_dquot_acct {
> struct xfs_dqtrx dqs[XFS_QM_TRANS_DQTYPES][XFS_QM_TRANS_MAXDQS];
> }
>
>
> and that makes all these loops something like:
>
> for (j = 0; j < XFS_QM_TRANS_DQTYPES; j++) {
> oqa = &otp->t_dqinfo->dqs[j];
>
> for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
> ....
> }
> }
>
> And all this nastiness of selecting the next structure element goes
> away.
>
> > STATIC xfs_dqtrx_t *
> > @@ -178,15 +185,20 @@ xfs_trans_get_dqtrx(
> > int i;
> > xfs_dqtrx_t *qa;
> >
> > - qa = XFS_QM_ISUDQ(dqp) ?
> > - tp->t_dqinfo->dqa_usrdquots : tp->t_dqinfo->dqa_grpdquots;
> > + if (XFS_QM_ISUDQ(dqp))
> > + qa = tp->t_dqinfo->dqa_usrdquots;
>
> qa = &tp->t_dqinfo->dqs[XFS_QM_TRANS_USR];
>
> > + else if (XFS_QM_ISGDQ(dqp))
> > + qa = tp->t_dqinfo->dqa_grpdquots;
>
> qa = &tp->t_dqinfo->dqs[XFS_QM_TRANS_GRP];
>
> > + else if (XFS_QM_ISPDQ(dqp))
> > + qa = tp->t_dqinfo->dqa_prjdquots;
>
> qa = &tp->t_dqinfo->dqs[XFS_QM_TRANS_PROJ];
>
> > + else
> > + return NULL;
> >
> > for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
> > if (qa[i].qt_dquot == NULL ||
> > qa[i].qt_dquot == dqp)
> > return &qa[i];
> > }
> > -
> > return NULL;
> > }
> >
> > @@ -340,9 +352,12 @@ xfs_trans_apply_dquot_deltas(
> >
> > ASSERT(tp->t_dqinfo);
> > qa = tp->t_dqinfo->dqa_usrdquots;
> > - for (j = 0; j < 2; j++) {
> > + for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
>
> Comments aren't necessary like this if the above enum/array
> construct is used.
>
> > if (qa[0].qt_dquot == NULL) {
> > - qa = tp->t_dqinfo->dqa_grpdquots;
> > + if (qa == tp->t_dqinfo->dqa_usrdquots)
> > + qa = tp->t_dqinfo->dqa_grpdquots;
> > + else
> > + qa = tp->t_dqinfo->dqa_prjdquots;
> > continue;
> > }
> >
> > @@ -496,9 +511,12 @@ xfs_trans_apply_dquot_deltas(
> > be64_to_cpu(dqp->q_core.d_rtbcount));
> > }
> > /*
> > - * Do the group quotas next
> > + * Do the group quotas or project quotas next
> > */
> > - qa = tp->t_dqinfo->dqa_grpdquots;
> > + if (qa == tp->t_dqinfo->dqa_usrdquots)
> > + qa = tp->t_dqinfo->dqa_grpdquots;
> > + else
> > + qa = tp->t_dqinfo->dqa_prjdquots;
> > }
> > }
> >
> > @@ -523,7 +541,7 @@ xfs_trans_unreserve_and_mod_dquots(
> >
> > qa = tp->t_dqinfo->dqa_usrdquots;
> >
> > - for (j = 0; j < 2; j++) {
> > + for (j = 0; j < 3; j++) { /* 0 - usr, 1 - grp, 2 - prj */
> > for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
> > qtrx = &qa[i];
> > /*
> > @@ -565,7 +583,10 @@ xfs_trans_unreserve_and_mod_dquots(
> > xfs_dqunlock(dqp);
> >
> > }
> > - qa = tp->t_dqinfo->dqa_grpdquots;
> > + if (qa == tp->t_dqinfo->dqa_usrdquots)
> > + qa = tp->t_dqinfo->dqa_grpdquots;
> > + else
> > + qa = tp->t_dqinfo->dqa_prjdquots;
> > }
>
> And all this repeated nastiness goes away...
>
> > }
> >
> > @@ -734,8 +755,8 @@ error_return:
> >
> > /*
> > * Given dquot(s), make disk block and/or inode reservations against them.
> > - * The fact that this does the reservation against both the usr and
> > - * grp/prj quotas is important, because this follows a both-or-nothing
> > + * The fact that this does the reservation against user, group and
> > + * project quotas is important, because this follows a all-or-nothing
> > * approach.
> > *
> > * flags = XFS_QMOPT_FORCE_RES evades limit enforcement. Used by chown.
> > @@ -750,6 +771,7 @@ xfs_trans_reserve_quota_bydquots(
> > xfs_mount_t *mp,
> > xfs_dquot_t *udqp,
> > xfs_dquot_t *gdqp,
> > + xfs_dquot_t *pdqp,
> > long nblks,
> > long ninos,
> > uint flags)
>
> kill the typedefs.
>
> > @@ -787,6 +809,24 @@ xfs_trans_reserve_quota_bydquots(
> > }
> > }
> >
> > + if (pdqp) {
> > + error = xfs_trans_dqresv(tp, mp, pdqp, nblks, ninos, flags);
> > + if (error) {
> > + /*
> > + * can't do it, so backout previous reservation
> > + */
> > + if (resvd) {
> > + flags |= XFS_QMOPT_FORCE_RES;
> > + xfs_trans_dqresv(tp, mp, udqp,
> > + -nblks, -ninos, flags);
> > + if (gdqp)
> > + xfs_trans_dqresv(tp, mp, gdqp,
> > + -nblks, -ninos, flags);
> > + }
> > + return error;
> > + }
> > + }
> > +
>
> This will only unwind group reservation is there was a user quota
> reservation. I think that is wrong.
>
> I think an unwind stack is better than the nested error
> handling, and it removes the need for the "resvd" variable to
> indicate that a user quota modification was made. i.e.
>
> if (udqp) {
> ....
> if (error)
> return error;
> ....
> }
> if (gdqp) {
> ....
> if (error)
> goto unwind_usr;
> ....
> }
> if (pdqp) {
> ....
> if (error)
> goto unwind_grp;
> ....
> }
>
>
> unwind_grp:
> flags |= XFS_QMOPT_FORCE_RES
> if (gdqp)
> xfs_trans_dqresv(tp, mp, gdqp, -nblks, -ninos, flags);
> unwind_usr:
> flags |= XFS_QMOPT_FORCE_RES
> if (udqp)
> xfs_trans_dqresv(tp, mp, udqp, -nblks, -ninos, flags);
> return error;
>
> > @@ -1498,7 +1502,7 @@ xfs_symlink(
> > int n;
> > xfs_buf_t *bp;
> > prid_t prid;
> > - struct xfs_dquot *udqp, *gdqp;
> > + struct xfs_dquot *udqp = NULL, *gdqp = NULL, *pdqp = NULL;
>
> One per line.
>
> Cheers,
>
> Dave.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 3/5] xfs: Add pquotaino to on-disk super block
2012-08-15 2:09 ` Dave Chinner
@ 2012-08-22 23:30 ` Chandra Seetharaman
2012-08-23 0:25 ` Dave Chinner
0 siblings, 1 reply; 19+ messages in thread
From: Chandra Seetharaman @ 2012-08-22 23:30 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
Thanks for the review...
Comments inline below...
On Wed, 2012-08-15 at 12:09 +1000, Dave Chinner wrote:
> On Fri, Jul 20, 2012 at 06:02:41PM -0500, Chandra Seetharaman wrote:
> > Add a new field to the superblock to add support for seperate pquota
> > with a specific version.
> >
> > Define a macro to differentiate superblock with and without OQUOTA.
> >
> > Keep backward compatibilty by alowing mount of older superblock
> > with no separate pquota inode.
> >
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > ---
> > fs/xfs/xfs_itable.c | 3 +-
> > fs/xfs/xfs_mount.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-
> > fs/xfs/xfs_qm.c | 18 +++++++++----
> > fs/xfs/xfs_qm_syscalls.c | 30 +++++++++++++++++-----
> > fs/xfs/xfs_quota.h | 8 ------
> > fs/xfs/xfs_sb.h | 20 ++++++++++++---
> > fs/xfs/xfs_super.c | 15 +++++++----
> > fs/xfs/xfs_trans_dquot.c | 4 ++-
> > include/linux/dqblk_xfs.h | 1 +
> > 9 files changed, 123 insertions(+), 34 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> > index eff577a..0fa60b4 100644
> > --- a/fs/xfs/xfs_itable.c
> > +++ b/fs/xfs/xfs_itable.c
> > @@ -42,7 +42,8 @@ xfs_internal_inum(
> > {
> > return (ino == mp->m_sb.sb_rbmino || ino == mp->m_sb.sb_rsumino ||
> > (xfs_sb_version_hasquota(&mp->m_sb) &&
> > - (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino)));
> > + (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino ||
> > + ino == mp->m_sb.sb_pquotino)));
> > }
> >
> > /*
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index 22f23fd..992ec29 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -108,6 +108,7 @@ static const struct {
> > { offsetof(xfs_sb_t, sb_logsunit), 0 },
> > { offsetof(xfs_sb_t, sb_features2), 0 },
> > { offsetof(xfs_sb_t, sb_bad_features2), 0 },
> > + { offsetof(xfs_sb_t, sb_pquotino), 0 },
>
> Can you line up the "0 }," part with the rest of the structure?
>
> > { sizeof(xfs_sb_t), 0 }
> > };
> >
> > @@ -618,6 +619,35 @@ 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);
> > +
> > + if (xfs_sb_version_has_no_oquota(to)) {
>
> <crash>
>
> My brain just core dumped on that name - "has no old quota"? "old
> quota" could mean anything, and it doesn't describe the change in
> on-disk format being made.
>
> The feature being added is a "separate project quota inode", so the
> feature bit and related functions need to reflect that. Hence I
> think a better name is something like "has project quota inode" i.e
> XFS_SB_VERSION2_PQUOTINO, xfs_sb_version_has_pquotino() and so on,
> which matches the above addition to the superblock fields...
will change it to refer pquotino
>
> > + if (to->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> > + xfs_notice(mp, "Super block has XFS_OQUOTA bits with "
> > + "version NO_OQUOTA. Fixing it.\n");
> > + to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> > + }
> > + to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
>
> So we have a feature bit set, but old quota bits set. How can that
> happen?
>
> If it does occur, doesn't that mean we cannot trust the group or
> project quotas to be correct, so at minimum this case needs to
> trigger a quotacheck for both group and project quotas?
Sure, will do a quotacheck here. I just call xfs_qm_quotacheck() and
fail if it fails ?
>
> And if we can't enumerate how it can happen, then should we even
> allow it to be "fixed" automatically?
>
> > + } else {
> > + if (to->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> > + XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
> > + xfs_notice(mp, "Super block has XFS_[G|P]UOTA bits in "
> > + "older version. Fixing it.\n");
> > + to->sb_qflags &= ~(XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> > + XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD);
>
> Again, how can that happen, and why isn't it a sign of corruption
> that requires external intervention? If those bits are set, we can't
> trust the quota to be correct, so at minimum we need to force a
> quota check.
same as above.
>
> > + }
> > + if (to->sb_qflags & XFS_OQUOTA_ENFD)
> > + to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
> > + XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
> > + if (to->sb_qflags & XFS_OQUOTA_CHKD)
> > + to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
> > + XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
>
> what do you do if both XFS_PQUOTA_ACCT and XFS_GQUOTA_ACCT are set?
> i.e. both quotas were active even though the feature bit wasn't set?
I will do a check on both flags being set and do a quotacheck ?
>
> This "fix it automatically" issue seems to be problematic to me
> because it isn't clear whether we can get into these states, or if
> we do, what the correct resolution of the problem is....
>
>
> > + to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> > +
> > + if (to->sb_qflags & XFS_PQUOTA_ACCT) {
> > + to->sb_pquotino = to->sb_gquotino;
> > + to->sb_gquotino = NULLFSINO;
> > + }
>
> This needs a comment explaining that the in-memory code will use the
> sb_pquotino for project quotas (i.e. behave as though the feature
> bit is always set), so we ahve to set up the in-memory image that
> way and convert it back to the old format when writing out the
> superblock.
will do.
>
> > + }
> > }
> >
> > /*
> > @@ -637,11 +667,22 @@ xfs_sb_to_disk(
> > int first;
> > int size;
> > __uint16_t tmp16;
> > + xfs_ino_t gquotino;
> >
> > ASSERT(fields);
> > if (!fields)
> > return;
> >
> > + /*
> > + * On-disk version earlier than NO_OQUOTA doesn't have sb_pquotino.
> > + * so, we need to copy the value to gquotino field.
> > + */
> > + if (!xfs_sb_version_has_no_oquota(from) &&
>
> <boom>
>
> Double negative! (not has no old quota).
>
> Another reason for naming it PQUOTINO....
>
> > + (from->sb_qflags & (XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD)))
> > + gquotino = from->sb_pquotino;
> > + else
> > + gquotino = from->sb_gquotino;
>
> Also, only do this transformation if we are modifying the group
> quota inode.....
>
will move to code to below.... (1)
> > +
> > while (fields) {
> > f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
> > first = xfs_sb_info[f].offset;
> > @@ -651,7 +692,8 @@ xfs_sb_to_disk(
> >
> > if (size == 1 || xfs_sb_info[f].type == 1) {
> > memcpy(to_ptr + first, from_ptr + first, size);
> > - } else if (f == XFS_SBS_QFLAGS) {
> > + } else if ((f == XFS_SBS_QFLAGS) &&
>
> [ no need for the extra (). ]
>
> > + !xfs_sb_version_has_no_oquota(from)) {
> > /*
> > * The in-core version of sb_qflags do not have
> > * XFS_OQUOTA_* flags, whereas the on-disk version
> > @@ -671,6 +713,8 @@ xfs_sb_to_disk(
> > tmp16 |= XFS_OQUOTA_CHKD;
> >
> > *(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
> > + } else if (f == XFS_SBS_GQUOTINO) {
> > + *(__be64 *)(to_ptr + first) = cpu_to_be64(gquotino);
>
> .... i.e. here.
(1) here, as suggested.
>
> > } else {
> > switch (size) {
> > case 2:
> > @@ -759,6 +803,12 @@ reread:
> > goto reread;
> > }
> >
> > + if (!xfs_sb_version_has_no_oquota(&mp->m_sb) &&
> > + XFS_IS_PQUOTA_ON(mp)) {
> > + mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
> > + mp->m_sb.sb_gquotino = NULLFSINO;
> > + }
>
> why this is necessary? Didn't the earlier xfs_sb_from_disk() call
> deal with this?
The call in xfs_sb_from_disk() only sets if the superblock has pquota
already.
This sets up the fields when superblock didn't have it, but the user
specified pquota as a mount option.
>
> > +
> > /* Initialize per-cpu counters */
> > xfs_icsb_reinit_counters(mp);
> >
> > @@ -1646,6 +1696,12 @@ xfs_mod_sb(xfs_trans_t *tp, __int64_t fields)
> > first = sizeof(xfs_sb_t);
> > last = 0;
> >
> > + if (!xfs_sb_version_has_no_oquota(&mp->m_sb) &&
> > + XFS_IS_PQUOTA_ON(mp)) {
> > + fields &= (__int64_t)~XFS_SB_PQUOTINO;
> > + fields |= (__int64_t)XFS_SB_GQUOTINO;
> > + }
>
> This will set the XFS_SB_GQUOTINO unconditionally. It only needs to
> be set this if the XFS_SB_PQUOTINO field is set.
will fix it.
>
> > @@ -838,19 +840,22 @@ xfs_qm_qino_alloc(
> > ASSERT((sbfields & (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> > XFS_SB_GQUOTINO | XFS_SB_QFLAGS)) ==
> > (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> > - XFS_SB_GQUOTINO | XFS_SB_QFLAGS));
> > + XFS_SB_GQUOTINO | XFS_SB_PQUOTINO | XFS_SB_QFLAGS));
>
> Did you test this with CONFIG_XFS_DEBUG=y? It will always fail
> because you didn't add XFS_SB_PQUOTINO to the sbfields mask....
In my box, I always had problems with DEBUG :(... So, I stopped testing
with it.
will fix it.
>
> > @@ -1156,7 +1161,8 @@ xfs_qm_dqusage_adjust(
> > * rootino must have its resources accounted for, not so with the quota
> > * inodes.
> > */
> > - if (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino) {
> > + if (ino == mp->m_sb.sb_uquotino || ino == mp->m_sb.sb_gquotino ||
> > + ino == mp->m_sb.sb_pquotino) {
>
> if (ino == mp->m_sb.sb_uquotino ||
> ino == mp->m_sb.sb_gquotino ||
> ino == mp->m_sb.sb_pquotino) {
>
> > @@ -413,17 +414,18 @@ xfs_qm_scall_getqstat(
> > struct fs_quota_stat *out)
> > {
> > struct xfs_quotainfo *q = mp->m_quotainfo;
> > - struct xfs_inode *uip, *gip;
> > - boolean_t tempuqip, tempgqip;
> > + struct xfs_inode *uip, *gip, *pip;
> > + boolean_t tempuqip, tempgqip, temppqip;
> >
> > - uip = gip = NULL;
> > - tempuqip = tempgqip = B_FALSE;
> > + uip = gip = pip = NULL;
> > + tempuqip = tempgqip = temppqip = B_FALSE;
>
> Line per declaration and initialisation.
>
> > diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
> > index 8fd7894..7373108 100644
> > --- a/fs/xfs/xfs_sb.h
> > +++ b/fs/xfs/xfs_sb.h
> > @@ -81,11 +81,15 @@ struct xfs_mount;
> > #define XFS_SB_VERSION2_ATTR2BIT 0x00000008 /* Inline attr rework */
> > #define XFS_SB_VERSION2_PARENTBIT 0x00000010 /* parent pointers */
> > #define XFS_SB_VERSION2_PROJID32BIT 0x00000080 /* 32 bit project id */
> > +#define XFS_SB_VERSION2_NO_OQUOTA 0x00000100 /* No OQUOTA and *
> > + * separate project *
> > + * quota field */
>
> see my comments about naming this above.
>
> > @@ -250,7 +255,7 @@ typedef enum {
> > XFS_SBS_GQUOTINO, XFS_SBS_QFLAGS, XFS_SBS_FLAGS, XFS_SBS_SHARED_VN,
> > XFS_SBS_INOALIGNMT, XFS_SBS_UNIT, XFS_SBS_WIDTH, XFS_SBS_DIRBLKLOG,
> > XFS_SBS_LOGSECTLOG, XFS_SBS_LOGSECTSIZE, XFS_SBS_LOGSUNIT,
> > - XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2,
> > + XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2, XFS_SBS_PQUOTINO,
>
> You got the name right here ;)
>
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 595f5ac..b475057 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -399,12 +399,6 @@ xfs_parseargs(
> > }
> > #endif
> >
> > - if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
> > - (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE))) {
> > - xfs_warn(mp, "cannot mount with both project and group quota");
> > - return EINVAL;
> > - }
> > -
> > if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
> > xfs_warn(mp, "sunit and swidth must be specified together");
> > return EINVAL;
> > @@ -1330,6 +1324,15 @@ xfs_fs_fill_super(
> > if (error)
> > goto out_destroy_counters;
> >
> > + if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
> > + (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE)) &&
> > + !xfs_sb_version_has_no_oquota(&mp->m_sb)) {
> > + xfs_warn(mp, "Super block does not support "
> > + "project and group quota together");
> > + error = EINVAL;
> > + goto out_free_sb;
> > + }
>
> This check belongs in xfs_finish_flags().
will move it.
>
> Cheers,
>
> Dave.
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 4/5] xfs: Add proper versioning support to fs_quota_stat
2012-08-15 2:32 ` Dave Chinner
@ 2012-08-22 23:32 ` Chandra Seetharaman
0 siblings, 0 replies; 19+ messages in thread
From: Chandra Seetharaman @ 2012-08-22 23:32 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
Will change as suggested.
Thanks for the review and clarification.
On Wed, 2012-08-15 at 12:32 +1000, Dave Chinner wrote:
> On Fri, Jul 20, 2012 at 06:02:47PM -0500, Chandra Seetharaman wrote:
> > Add proper versioning support for fs_quota_stat.
> >
> > Leave the earlier version as version 1 and add version 2 to add a new
> > field "fs_qfilestat_t qs_pquota"
> >
> > Callers of the system call have to just set the version of the data
> > structure being passed, and kernel will fill as much data as possible.
>
> Userspace Interface Traps for the Unwary 101, below.
>
> > /*
> > * Disk quota - quotactl(2) commands for the XFS Quota Manager (XQM).
> > @@ -139,6 +140,7 @@ typedef struct fs_disk_quota {
> > * incore.
> > */
> > #define FS_QSTAT_VERSION 1 /* fs_quota_stat.qs_version */
> > +#define FS_QSTAT_VERSION_2 2 /* new field qs_pquota */
> >
> > /*
> > * Some basic information about 'quota files'.
> > @@ -155,13 +157,37 @@ typedef struct fs_quota_stat {
> > __s8 qs_pad; /* unused */
> > fs_qfilestat_t qs_uquota; /* user quota storage information */
> > fs_qfilestat_t qs_gquota; /* group quota storage information */
> > -#define qs_pquota qs_gquota
> > __u32 qs_incoredqs; /* number of dquots incore */
> > __s32 qs_btimelimit; /* limit for blks timer */
> > __s32 qs_itimelimit; /* limit for inodes timer */
> > __s32 qs_rtbtimelimit;/* limit for rt blks timer */
> > __u16 qs_bwarnlimit; /* limit for num warnings */
> > __u16 qs_iwarnlimit; /* limit for num warnings */
> > + fs_qfilestat_t qs_pquota; /* project quota storage information */
> > } fs_quota_stat_t;
> >
> > +#define FS_QSTAT_V1_SIZE (offsetof(fs_quota_stat_t, qs_pquota))
> > +#define FS_QSTAT_V2_SIZE (FS_QSTAT_V1_SIZE + sizeof (fs_qfilestat_t))
>
> I don't expect that will work on all arches. pahole (everyone needs
> to know about pahole!) tells me the original structure on x86_64
> looks like:
>
> struct fs_quota_stat {
> __s8 qs_version; /* 0 1 */
>
> /* XXX 1 byte hole, try to pack */
>
> __u16 qs_flags; /* 2 2 */
> __s8 qs_pad; /* 4 1 */
>
> /* XXX 3 bytes hole, try to pack */
>
> fs_qfilestat_t qs_uquota; /* 8 24 */
> fs_qfilestat_t qs_gquota; /* 32 24 */
> __u32 qs_incoredqs; /* 56 4 */
> __s32 qs_btimelimit; /* 60 4 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> __s32 qs_itimelimit; /* 64 4 */
> __s32 qs_rtbtimelimit; /* 68 4 */
> __u16 qs_bwarnlimit; /* 72 2 */
> __u16 qs_iwarnlimit; /* 74 2 */
>
> /* size: 80, cachelines: 2, members: 11 */
> /* sum members: 72, holes: 2, sum holes: 4 */
> /* padding: 4 */
> /* last cacheline: 16 bytes */
> }
>
> and that qs_iwarnlimit does not end on a 8 byte boundary. If we then
> look at fs_qfilestat:
>
> typedef struct fs_qfilestat {
> __u64 qfs_ino; /* inode number */
> __u64 qfs_nblks; /* number of BBs 512-byte-blks */
> __u32 qfs_nextents; /* number of extents */
> } fs_qfilestat_t;
>
> it has an 8 byte alignment, so the above FS_QSTAT_V2_SIZE
> calculation is wrong because it doesn't take into account holes in
> the structure and there is 4 bytes of hole between qs_iwarnlimit and
> qs_pquota. It's entirely possible that this might require a compat
> handler as some platforms might pack the structure differently in 32
> vs 64 bit binaries..
>
> IOWs, you ned to explicitly add padding to thie structure, like
> someone tried to do in the past a screwed it up completely.
> Basically, the structure needs to be padded like this:
>
> typedef struct fs_quota_stat {
> __s8 qs_version; /* version number for future changes */
> __u8 qs_pad1; /* unused */
> __u16 qs_flags; /* FS_QUOTA_{U,P,G}DQ_{ACCT,ENFD} */
> __u8 qs_pad2[3]; /* unused */
> fs_qfilestat_t qs_uquota; /* user quota storage information */
> fs_qfilestat_t qs_gquota; /* group quota storage information */
> __u32 qs_incoredqs; /* number of dquots incore */
> __s32 qs_btimelimit; /* limit for blks timer */
> __s32 qs_itimelimit; /* limit for inodes timer */
> __s32 qs_rtbtimelimit;/* limit for rt blks timer */
> __u16 qs_bwarnlimit; /* limit for num warnings */
> __u16 qs_iwarnlimit; /* limit for num warnings */
> __u8 qs_pad3[4]; /* unused */
> fs_qfilestat_t qs_pquota; /* project quota storage information */
> } fs_quota_stat_t;
>
> > +
> > +static inline int valid_qstat_version(int version)
> > +{
> > + switch (version) {
> > + case FS_QSTAT_VERSION:
> > + case FS_QSTAT_VERSION_2:
> > + return 1;
> > + default:
> > + return 0;
> > + }
> > +}
> > +static inline int qstatsize(int version)
> > +{
> > + switch (version) {
> > + case FS_QSTAT_VERSION_2:
> > + return FS_QSTAT_V2_SIZE;
> > + case FS_QSTAT_VERSION:
> > + default:
> > + return FS_QSTAT_V1_SIZE;
> > + }
> > +}
>
> I don't see much point in these helpers - just put them in line in
> the quota_getxstate() code. i.e.
>
> switch (version) {
> case FS_QSTAT_VERSION_2:
> size = FS_QSTAT_V2_SIZE;
> break;
> default:
> version = FS_QSTAT_VERSION;
> /* fallthrough */
> case FS_QSTAT_VERSION:
> size = FS_QSTAT_V1_SIZE;
> break;
> }
>
> Cheers,
>
> Dave.
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 5/5] xfs: Add a new field to fs_quota_stat to get pquota information
2012-08-15 2:37 ` Dave Chinner
@ 2012-08-22 23:40 ` Chandra Seetharaman
0 siblings, 0 replies; 19+ messages in thread
From: Chandra Seetharaman @ 2012-08-22 23:40 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
Thanks for the review..
On Wed, 2012-08-15 at 12:37 +1000, Dave Chinner wrote:
> On Fri, Jul 20, 2012 at 06:03:05PM -0500, Chandra Seetharaman wrote:
> > Use separate structure for filling the project quota information
> > for the Q_XGETQSTAT quota command.
> >
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > ---
> > fs/xfs/xfs_qm_syscalls.c | 26 +++++++++++++-------------
> > 1 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> > index f011cf3..482efc0 100644
> > --- a/fs/xfs/xfs_qm_syscalls.c
> > +++ b/fs/xfs/xfs_qm_syscalls.c
> > @@ -421,7 +421,6 @@ xfs_qm_scall_getqstat(
> > tempuqip = tempgqip = temppqip = B_FALSE;
> > memset(out, 0, sizeof(fs_quota_stat_t));
> >
> > - out->qs_version = FS_QSTAT_VERSION;
> > if (!xfs_sb_version_hasquota(&mp->m_sb)) {
> > out->qs_uquota.qfs_ino = NULLFSINO;
> > out->qs_gquota.qfs_ino = NULLFSINO;
> > @@ -434,8 +433,6 @@ xfs_qm_scall_getqstat(
> > out->qs_pad = 0;
> > out->qs_uquota.qfs_ino = mp->m_sb.sb_uquotino;
> > out->qs_gquota.qfs_ino = mp->m_sb.sb_gquotino;
> > - if (&out->qs_gquota != &out->qs_pquota)
> > - out->qs_pquota.qfs_ino = mp->m_sb.sb_pquotino;
> >
> > if (q) {
> > uip = q->qi_uquotaip;
> > @@ -452,11 +449,6 @@ xfs_qm_scall_getqstat(
> > 0, 0, &gip) == 0)
> > tempgqip = B_TRUE;
> > }
> > - if (!pip && mp->m_sb.sb_pquotino != NULLFSINO) {
> > - if (xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
> > - 0, 0, &pip) == 0)
> > - temppqip = B_TRUE;
> > - }
> > if (uip) {
> > out->qs_uquota.qfs_nblks = uip->i_d.di_nblocks;
> > out->qs_uquota.qfs_nextents = uip->i_d.di_nextents;
> > @@ -469,11 +461,19 @@ xfs_qm_scall_getqstat(
> > if (tempgqip)
> > IRELE(gip);
> > }
> > - if (pip) {
> > - out->qs_pquota.qfs_nblks = pip->i_d.di_nblocks;
> > - out->qs_pquota.qfs_nextents = pip->i_d.di_nextents;
> > - if (temppqip)
> > - IRELE(pip);
> > + if (out->qs_version >= FS_QSTAT_VERSION_2) {
> > + out->qs_pquota.qfs_ino = mp->m_sb.sb_pquotino;
> > + if (!pip && mp->m_sb.sb_pquotino != NULLFSINO) {
> > + if (xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
> > + 0, 0, &pip) == 0)
> > + temppqip = B_TRUE;
> > + }
> > + if (pip) {
> > + out->qs_pquota.qfs_nblks = pip->i_d.di_nblocks;
> > + out->qs_pquota.qfs_nextents = pip->i_d.di_nextents;
> > + if (temppqip)
> > + IRELE(pip);
> > + }
>
> Doesn't this mean that we'll never report project quota for
> FS_QSTAT_VERSION structures and the old quota format? i.e. existing
> tools with a new kernel won't return the project quota info in the
> group quota fields?
Good catch. I will add additional code to check the qs_version and the
new pquota flag to cover that combination.
>
> Cheers,
>
> Dave.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 3/5] xfs: Add pquotaino to on-disk super block
2012-08-22 23:30 ` Chandra Seetharaman
@ 2012-08-23 0:25 ` Dave Chinner
0 siblings, 0 replies; 19+ messages in thread
From: Dave Chinner @ 2012-08-23 0:25 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
On Wed, Aug 22, 2012 at 06:30:48PM -0500, Chandra Seetharaman wrote:
> On Wed, 2012-08-15 at 12:09 +1000, Dave Chinner wrote:
> > On Fri, Jul 20, 2012 at 06:02:41PM -0500, Chandra Seetharaman wrote:
.....
> > > + if (to->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> > > + xfs_notice(mp, "Super block has XFS_OQUOTA bits with "
> > > + "version NO_OQUOTA. Fixing it.\n");
> > > + to->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> > > + }
> > > + to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
> >
> > So we have a feature bit set, but old quota bits set. How can that
> > happen?
> >
> > If it does occur, doesn't that mean we cannot trust the group or
> > project quotas to be correct, so at minimum this case needs to
> > trigger a quotacheck for both group and project quotas?
>
> Sure, will do a quotacheck here. I just call xfs_qm_quotacheck() and
> fail if it fails ?
The quotacheck occurs later in the mount process. IIRC, just
clearing the relevant XFS_[UGP]QUOTA_CHKD flag will cause a quota
check to be done at the appropriate time.
> > > + }
> > > + if (to->sb_qflags & XFS_OQUOTA_ENFD)
> > > + to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
> > > + XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
> > > + if (to->sb_qflags & XFS_OQUOTA_CHKD)
> > > + to->sb_qflags |= (to->sb_qflags & XFS_PQUOTA_ACCT) ?
> > > + XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
> >
> > what do you do if both XFS_PQUOTA_ACCT and XFS_GQUOTA_ACCT are set?
> > i.e. both quotas were active even though the feature bit wasn't set?
>
> I will do a check on both flags being set and do a quotacheck ?
Yes, I think that will be sufficient with an appropriate warning.
> > > } else {
> > > switch (size) {
> > > case 2:
> > > @@ -759,6 +803,12 @@ reread:
> > > goto reread;
> > > }
> > >
> > > + if (!xfs_sb_version_has_no_oquota(&mp->m_sb) &&
> > > + XFS_IS_PQUOTA_ON(mp)) {
> > > + mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
> > > + mp->m_sb.sb_gquotino = NULLFSINO;
> > > + }
> >
> > why this is necessary? Didn't the earlier xfs_sb_from_disk() call
> > deal with this?
>
> The call in xfs_sb_from_disk() only sets if the superblock has pquota
> already.
>
> This sets up the fields when superblock didn't have it, but the user
> specified pquota as a mount option.
Ah, so it is the same as the previous case I mentioned needs a
comment. Can you document this here as well?
> > > @@ -838,19 +840,22 @@ xfs_qm_qino_alloc(
> > > ASSERT((sbfields & (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> > > XFS_SB_GQUOTINO | XFS_SB_QFLAGS)) ==
> > > (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> > > - XFS_SB_GQUOTINO | XFS_SB_QFLAGS));
> > > + XFS_SB_GQUOTINO | XFS_SB_PQUOTINO | XFS_SB_QFLAGS));
> >
> > Did you test this with CONFIG_XFS_DEBUG=y? It will always fail
> > because you didn't add XFS_SB_PQUOTINO to the sbfields mask....
>
> In my box, I always had problems with DEBUG :(... So, I stopped testing
> with it.
Hmmm. DEBUG shouldn't cause you any extra problems unless there
really is something wrong - I run almost exclusively with DEBUG
enabled and I rarely have problems with spurious ASSERT()s
triggering. It's better to report spurious/unrelated ASSERT()
failures than to ignore them.
[ FWIW, the only time I turnoff DEBUG is when I'm doing performance
benchmarking to get maximum numbers - DEBUG drops metadata
throughput by about 25% and changes allocation patterns to improve
code coverage, so the results of performance testing with DEBUG are
not really representative. ]
In the mean time, run with DEBUG without your patches and exclude
all the tests that trigger problems on a vanilla kernel (e.g. via
'echo 068 >> expunged') and then run with you patches. Any new
failures are likely to be caused by your patches and need to be
analysed.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD
2012-08-22 22:53 ` Chandra Seetharaman
@ 2012-09-13 7:56 ` Christoph Hellwig
2012-09-13 20:37 ` Chandra Seetharaman
0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2012-09-13 7:56 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
On Wed, Aug 22, 2012 at 05:53:22PM -0500, Chandra Seetharaman wrote:
> will fix them as suggested.
>
> Thanks for the review.
Any updates on reposting the series?
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD
2012-09-13 7:56 ` Christoph Hellwig
@ 2012-09-13 20:37 ` Chandra Seetharaman
0 siblings, 0 replies; 19+ messages in thread
From: Chandra Seetharaman @ 2012-09-13 20:37 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Thu, 2012-09-13 at 03:56 -0400, Christoph Hellwig wrote:
> On Wed, Aug 22, 2012 at 05:53:22PM -0500, Chandra Seetharaman wrote:
> > will fix them as suggested.
> >
> > Thanks for the review.
>
> Any updates on reposting the series?
Sorry, got busy with other things. Will get back to this next week.
Chandra
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-09-13 20:36 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-20 23:02 [RFC v6 PATCH 0/5] xfs: Allow pquota and gquota to be used together Chandra Seetharaman
2012-07-20 23:02 ` [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD Chandra Seetharaman
2012-08-14 22:46 ` Dave Chinner
2012-08-22 22:53 ` Chandra Seetharaman
2012-09-13 7:56 ` Christoph Hellwig
2012-09-13 20:37 ` Chandra Seetharaman
2012-07-20 23:02 ` [RFC v6 PATCH 2/5] xfs: Add pquota fields where gquota is used Chandra Seetharaman
2012-08-15 1:15 ` Dave Chinner
2012-08-22 22:56 ` Chandra Seetharaman
2012-07-20 23:02 ` [RFC v6 PATCH 3/5] xfs: Add pquotaino to on-disk super block Chandra Seetharaman
2012-08-15 2:09 ` Dave Chinner
2012-08-22 23:30 ` Chandra Seetharaman
2012-08-23 0:25 ` Dave Chinner
2012-07-20 23:02 ` [RFC v6 PATCH 4/5] xfs: Add proper versioning support to fs_quota_stat Chandra Seetharaman
2012-08-15 2:32 ` Dave Chinner
2012-08-22 23:32 ` Chandra Seetharaman
2012-07-20 23:03 ` [RFC v6 PATCH 5/5] xfs: Add a new field to fs_quota_stat to get pquota information Chandra Seetharaman
2012-08-15 2:37 ` Dave Chinner
2012-08-22 23:40 ` Chandra Seetharaman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.