[PATCH v11 0/4] Allow pquota and gquota to be used together

[PATCH v11 0/4] Allow pquota and gquota to be used together

[Chandra Seetharaman]

Hello All,

This is the version 11 of the changes to allow pquota and gquota to be used
together.

Patchset applies cleanly on top of 42c49d7f249c2487f36d3314753d5d8ebcee8249
in xfs git tree.

---

Chandra Seetharaman (4):
  xfs: Add pquota fields where gquota is used.
  xfs: Start using pquotaino from the superblock.
  xfs: Add proper versioning support to fs_quota_stat
  xfs: Use new qs_pquota field in fs_quota_stat for Q_XGETQSTAT

 fs/gfs2/quota.c                |    3 -
 fs/quota/quota.c               |   67 ++++++++++-
 fs/xfs/xfs_dquot.c             |   15 ++-
 fs/xfs/xfs_dquot.h             |    7 +-
 fs/xfs/xfs_icache.c            |    1 +
 fs/xfs/xfs_inode.h             |    1 +
 fs/xfs/xfs_ioctl.c             |   14 +-
 fs/xfs/xfs_iops.c              |    4 +-
 fs/xfs/xfs_mount.c             |   62 +++++++++--
 fs/xfs/xfs_qm.c                |  259 ++++++++++++++++++++++++++++------------
 fs/xfs/xfs_qm.h                |   20 ++-
 fs/xfs/xfs_qm_bhv.c            |   10 +-
 fs/xfs/xfs_qm_syscalls.c       |   70 +++++++++---
 fs/xfs/xfs_quota.h             |   32 +++--
 fs/xfs/xfs_sb.h                |    9 ++-
 fs/xfs/xfs_super.c             |   19 ++--
 fs/xfs/xfs_symlink.c           |   10 +-
 fs/xfs/xfs_trans_dquot.c       |   26 ++++-
 fs/xfs/xfs_vnodeops.c          |   13 ++-
 include/uapi/linux/dqblk_xfs.h |   59 ++++++++-
 20 files changed, 528 insertions(+), 173 deletions(-)
----------

Changes from version 10 to verion 11:
 - Few of the patches have been merged.
 - Localized all the logic to handle simultaneous use
   of XFS_SB_GQUOTINO and XFS_SB_PQUOTINO to xfs_sb_quota_from_disk()
   and xfs_sb_from_disk as per Dave's suggestion.
 - moved setting sb_pquotino to xfs_sb_quota_frp,_disk() as per
   Dave's suggestion.
 - Added new Q_XGETQSTATV command to handle the new sematics of
   quota_stata as per Jan's comment.
 - Fixed few nits as per Ben's comment

Version 10 can be found at:
http://oss.sgi.com/archives/xfs/2013-06/msg00902.html

Changes from version 9 to version 10:
 - Split prep into 6 patches as per Ben and Dave's suggestion
 - Added xfs_sb_quota_from_disk() as per Dave's suggestion
 - Made u/g/p ordering consistent
 - Fixed fields in fs_quota_stat as per Dave's suggestion
 - Moved few ASSERTs to be inside error check as per Ben's suggestion
 - Few cosmetic changes

Version 9 can be found at:
http://oss.sgi.com/archives/xfs/2013-06/msg00770.html

Changes from verion 8 to version 9:
 - Made all the changes suggested by Dave
 - Rearranged code to make the functional changes to
   not be cluttered.

Version 8 can be found at:
http://oss.sgi.com/archives/xfs/2013-05/msg00296.html

Changes from version 7 to version 8:
 - Rebased to the current code.
 - Modified code to use the new pquotino field in version 5
   of the superblock.

Version 7 can be found at:
http://oss.sgi.com/archives/xfs/2013-03/msg00671.html

Changes from version 6 to version 7:
 - Made all the changes suggested by Dave Chinner
 - Added padding to fs_quota_stat, which made the new
   data structure field ordering different from the old data
   structure field ordering. So, copied the old version
   to be _v1 version and used that if old version of data
   structure was provided from user space.

Version 6 can be found at:
http://oss.sgi.com/archives/xfs/2012-07/msg00286.html

Changes from version 5 to version 6:
- Use radix tree instead of hash table

Version 5 can be found at:
http://oss.sgi.com/archives/xfs/2012-03/msg00310.html

Changes fron version 4 to version 5:
rebase with the current tree one simple change.

Version 4 can be found at
http://oss.sgi.com/archives/xfs/2012-02/msg00528.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 3 of the posting can be found at
http://oss.sgi.com/archives/xfs/2012-01/msg00309.html

Changes from version 2 to version 3:
 - hash table for pquota is added.
 - changes to apply cleanly with the latest tree

version 2 of the posting can be found at
http://oss.sgi.com/archives/xfs/2011-10/msg00470.html

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()

version 1 of the posting can be found at
http://oss.sgi.com/archives/xfs/2011-10/msg00341.html

Thanks & Regards,

chandra

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

[PATCH v11 1/4] xfs: Add pquota fields where gquota is used.

[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.

Not using the pquotino field from superblock yet.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/xfs_dquot.c       |   15 +++-
 fs/xfs/xfs_dquot.h       |    7 +-
 fs/xfs/xfs_icache.c      |    1 +
 fs/xfs/xfs_inode.h       |    1 +
 fs/xfs/xfs_ioctl.c       |   14 ++--
 fs/xfs/xfs_iops.c        |    4 +-
 fs/xfs/xfs_qm.c          |  243 +++++++++++++++++++++++++++++++++-------------
 fs/xfs/xfs_qm.h          |   20 +++--
 fs/xfs/xfs_qm_bhv.c      |   10 +-
 fs/xfs/xfs_qm_syscalls.c |   19 +++-
 fs/xfs/xfs_quota.h       |   32 ++++---
 fs/xfs/xfs_symlink.c     |   10 ++-
 fs/xfs/xfs_trans_dquot.c |   26 ++++-
 fs/xfs/xfs_vnodeops.c    |   13 ++-
 14 files changed, 291 insertions(+), 124 deletions(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index f01012d..0adf27e 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -936,6 +936,7 @@ xfs_qm_dqput_final(
 {
 	struct xfs_quotainfo	*qi = dqp->q_mount->m_quotainfo;
 	struct xfs_dquot	*gdqp;
+	struct xfs_dquot	*pdqp;
 
 	trace_xfs_dqput_free(dqp);
 
@@ -949,21 +950,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 b596626..55abbca 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -53,6 +53,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 */
@@ -118,8 +119,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;
 	}
@@ -131,8 +133,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;
 	}
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 9560dc1..3f90e1c 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -337,6 +337,7 @@ xfs_iget_cache_miss(
 		iflags |= XFS_IDONTCACHE;
 	ip->i_udquot = NULL;
 	ip->i_gdquot = NULL;
+	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 9112979..b55fd34 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -250,6 +250,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 dc5b659..6e2bca5 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -928,7 +928,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;
 
@@ -957,7 +957,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;
 	}
@@ -994,8 +994,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;
@@ -1113,7 +1113,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);
 
@@ -1160,13 +1160,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 8865261..96dda62 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -539,7 +539,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;
 	}
@@ -575,7 +575,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 7a3e007..d320794 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -137,6 +137,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	*pdqp = NULL;
 
 	xfs_dqlock(dqp);
 	if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) {
@@ -145,8 +146,7 @@ xfs_qm_dqpurge(
 	}
 
 	/*
-	 * If this quota has a group hint attached, prepare for releasing it
-	 * now.
+	 * If this quota has a hint attached, prepare for releasing it now.
 	 */
 	gdqp = dqp->q_gdquot;
 	if (gdqp) {
@@ -154,6 +154,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);
@@ -208,6 +214,8 @@ xfs_qm_dqpurge(
 
 	if (gdqp)
 		xfs_qm_dqput(gdqp);
+	if (pdqp)
+		xfs_qm_dqput(pdqp);
 	return 0;
 }
 
@@ -364,6 +372,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;
+		}
 	}
 }
 
@@ -410,7 +422,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);
 
@@ -453,28 +468,42 @@ xfs_qm_dqattach_one(
 
 
 /*
- * Given a udquot and gdquot, attach a ptr to the group dquot in the
- * udquot as a hint for future lookups.
+ * Given a udquot and group/project type, attach the group/project
+ * dquot pointer to 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_hint(
+	struct xfs_inode	*ip,
+	int			type)
 {
-	xfs_dquot_t	*tmp;
+	struct xfs_dquot **dqhintp;
+	struct xfs_dquot *dqp;
+	struct xfs_dquot *udq = ip->i_udquot;
+
+	ASSERT(type == XFS_DQ_GROUP || type == XFS_DQ_PROJ);
 
 	xfs_dqlock(udq);
 
-	tmp = udq->q_gdquot;
-	if (tmp) {
-		if (tmp == gdq)
+	if (type == XFS_DQ_GROUP) {
+		dqp = ip->i_gdquot;
+		dqhintp = &udq->q_gdquot;
+	} else {
+		dqp = ip->i_pdquot;
+		dqhintp = &udq->q_pdquot;
+	}
+
+	if (*dqhintp) {
+		struct xfs_dquot *tmp;
+
+		if (*dqhintp == dqp)
 			goto done;
 
-		udq->q_gdquot = NULL;
+		tmp = *dqhintp;
+		*dqhintp = NULL;
 		xfs_qm_dqrele(tmp);
 	}
 
-	udq->q_gdquot = xfs_qm_dqhold(gdq);
+	*dqhintp = xfs_qm_dqhold(dqp);
 done:
 	xfs_dqunlock(udq);
 }
@@ -527,12 +556,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);
 		/*
@@ -544,14 +569,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
@@ -560,7 +599,10 @@ xfs_qm_dqattach_locked(
 		 * succeed in general.
 		 */
 		if (ip->i_udquot->q_gdquot != ip->i_gdquot)
-			xfs_qm_dqattach_grouphint(ip->i_udquot, ip->i_gdquot);
+			xfs_qm_dqattach_hint(ip, XFS_DQ_GROUP);
+
+		if (ip->i_udquot->q_pdquot != ip->i_pdquot)
+			xfs_qm_dqattach_hint(ip, XFS_DQ_PROJ);
 	}
 
  done:
@@ -568,8 +610,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
@@ -602,7 +646,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 +660,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;
+	}
 }
 
 int
@@ -660,6 +708,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);
@@ -761,6 +810,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;
@@ -1269,13 +1322,14 @@ xfs_qm_quotacheck(
 	LIST_HEAD		(buffer_list);
 	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;
 
 	count = INT_MAX;
 	structsz = 1;
 	lastino = 0;
 	flags = 0;
 
-	ASSERT(uip || gip);
+	ASSERT(uip || gip || pip);
 	ASSERT(XFS_IS_QUOTA_RUNNING(mp));
 
 	xfs_notice(mp, "Quotacheck needed: Please wait.");
@@ -1294,13 +1348,19 @@ xfs_qm_quotacheck(
 	}
 
 	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;
+	}
+
+	if (pip) {
+		error = xfs_qm_dqiterate(mp, pip, XFS_QMOPT_PQUOTA,
+					 &buffer_list);
+		if (error)
+			goto error_return;
+		flags |= XFS_PQUOTA_CHKD;
 	}
 
 	do {
@@ -1397,6 +1457,7 @@ xfs_qm_init_quotainos(
 {
 	struct xfs_inode	*uip = NULL;
 	struct xfs_inode	*gip = NULL;
+	struct xfs_inode	*pip = NULL;
 	int			error;
 	__int64_t		sbflags = 0;
 	uint			flags = 0;
@@ -1415,7 +1476,7 @@ xfs_qm_init_quotainos(
 			if (error)
 				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);
 			error = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino,
@@ -1423,6 +1484,15 @@ xfs_qm_init_quotainos(
 			if (error)
 				goto error_rele;
 		}
+		/* XXX: Use gquotino for now */
+		if (XFS_IS_PQUOTA_ON(mp) &&
+		    mp->m_sb.sb_gquotino != NULLFSINO) {
+			ASSERT(mp->m_sb.sb_gquotino > 0);
+			error = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino,
+					     0, 0, &pip);
+			if (error)
+				goto error_rele;
+		}
 	} else {
 		flags |= XFS_QMOPT_SBVERSION;
 		sbflags |= (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
@@ -1430,7 +1500,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.
@@ -1444,17 +1514,27 @@ 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)
+			goto error_rele;
+
+		flags &= ~XFS_QMOPT_SBVERSION;
+	}
+	if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
+		/* XXX: Use XFS_SB_GQUOTINO for now */
+		error = xfs_qm_qino_alloc(mp, &pip,
+					  sbflags | XFS_SB_GQUOTINO,
+					  flags | XFS_QMOPT_PQUOTA);
 		if (error)
 			goto error_rele;
 	}
 
 	mp->m_quotainfo->qi_uquotaip = uip;
 	mp->m_quotainfo->qi_gquotaip = gip;
+	mp->m_quotainfo->qi_pquotaip = pip;
 
 	return 0;
 
@@ -1463,6 +1543,8 @@ error_rele:
 		IRELE(uip);
 	if (gip)
 		IRELE(gip);
+	if (pip)
+		IRELE(pip);
 	return XFS_ERROR(error);
 }
 
@@ -1657,11 +1739,13 @@ 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 = NULL;
 	struct xfs_dquot	*gq = NULL;
+	struct xfs_dquot	*pq = NULL;
 	int			error;
 	uint			lockflags;
 
@@ -1741,24 +1825,25 @@ 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);
 			error = xfs_qm_dqget(mp, NULL, (xfs_dqid_t)prid,
 						 XFS_DQ_PROJ,
 						 XFS_QMOPT_DQALLOC |
 						 XFS_QMOPT_DOWARN,
-						 &gq);
+						 &pq);
 			if (error) {
 				ASSERT(error != ENOENT);
 				goto error_rele;
 			}
-			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)
@@ -1773,9 +1858,15 @@ 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;
 
 error_rele:
+	if (gq)
+		xfs_qm_dqrele(gq);
 	if (uq)
 		xfs_qm_dqrele(uq);
 	return error;
@@ -1830,14 +1921,17 @@ xfs_qm_vop_chown_reserve(
 	struct xfs_inode	*ip,
 	struct xfs_dquot	*udqp,
 	struct xfs_dquot	*gdqp,
+	struct xfs_dquot	*pdqp,
 	uint			flags)
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	uint			delblks, blkflags, prjflags = 0;
 	struct xfs_dquot	*udq_unres = NULL;
 	struct xfs_dquot	*gdq_unres = NULL;
+	struct xfs_dquot	*pdq_unres = NULL;
 	struct xfs_dquot	*udq_delblks = NULL;
 	struct xfs_dquot	*gdq_delblks = NULL;
+	struct xfs_dquot	*pdq_delblks = NULL;
 	int			error;
 
 
@@ -1861,24 +1955,28 @@ xfs_qm_vop_chown_reserve(
 			udq_unres = 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))) {
-			gdq_delblks = gdqp;
-			if (delblks) {
-				ASSERT(ip->i_gdquot);
-				gdq_unres = 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)) {
+		gdq_delblks = gdqp;
+		if (delblks) {
+			ASSERT(ip->i_gdquot);
+			gdq_unres = 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;
+		pdq_delblks = pdqp;
+		if (delblks) {
+			ASSERT(ip->i_pdquot);
+			pdq_unres = ip->i_pdquot;
 		}
 	}
 
 	error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
-				udq_delblks, gdq_delblks, ip->i_d.di_nblocks, 1,
+				udq_delblks, gdq_delblks, pdq_delblks,
+				ip->i_d.di_nblocks, 1,
 				flags | blkflags | prjflags);
 	if (error)
 		return error;
@@ -1893,16 +1991,17 @@ xfs_qm_vop_chown_reserve(
 		/*
 		 * Do the reservations first. Unreservation can't fail.
 		 */
-		ASSERT(udq_delblks || gdq_delblks);
-		ASSERT(udq_unres || gdq_unres);
+		ASSERT(udq_delblks || gdq_delblks || pdq_delblks);
+		ASSERT(udq_unres || gdq_unres || pdq_unres);
 		error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
-			    udq_delblks, gdq_delblks, (xfs_qcnt_t)delblks, 0,
+			    udq_delblks, gdq_delblks, pdq_delblks,
+			    (xfs_qcnt_t)delblks, 0,
 			    flags | blkflags | prjflags);
 		if (error)
 			return error;
 		xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
-				udq_unres, gdq_unres, -((xfs_qcnt_t)delblks), 0,
-				blkflags);
+				udq_unres, gdq_unres, pdq_unres,
+				-((xfs_qcnt_t)delblks), 0, blkflags);
 	}
 
 	return (0);
@@ -1941,7 +2040,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;
 
@@ -1961,13 +2061,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_IS_GQUOTA_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 bdb4f8b..579d6a0 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 */
+	struct xfs_inode	*qi_uquotaip;	/* user quota inode */
+	struct xfs_inode	*qi_gquotaip;	/* group quota inode */
+	struct xfs_inode	*qi_pquotaip;	/* project quota inode */
 	struct list_head qi_lru_list;
 	struct mutex	 qi_lru_lock;
 	int		 qi_lru_count;
@@ -78,8 +80,9 @@ xfs_dquot_tree(
 	case XFS_DQ_USER:
 		return &qi->qi_uquota_tree;
 	case XFS_DQ_GROUP:
-	case XFS_DQ_PROJ:
 		return &qi->qi_gquota_tree;
+	case XFS_DQ_PROJ:
+		return &qi->qi_pquota_tree;
 	default:
 		ASSERT(0);
 	}
@@ -93,8 +96,9 @@ xfs_dq_to_quota_inode(struct xfs_dquot *dqp)
 	case XFS_DQ_USER:
 		return dqp->q_mount->m_quotainfo->qi_uquotaip;
 	case XFS_DQ_GROUP:
-	case XFS_DQ_PROJ:
 		return dqp->q_mount->m_quotainfo->qi_gquotaip;
+	case XFS_DQ_PROJ:
+		return dqp->q_mount->m_quotainfo->qi_pquotaip;
 	default:
 		ASSERT(0);
 	}
@@ -107,18 +111,20 @@ extern void	xfs_trans_mod_dquot(struct xfs_trans *,
 					struct xfs_dquot *, uint, long);
 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 void	xfs_trans_dqjoin(struct xfs_trans *, struct xfs_dquot *);
 extern void	xfs_trans_log_dquot(struct xfs_trans *, struct xfs_dquot *);
 
 /*
- * 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.
  */
 enum {
 	XFS_QM_TRANS_USR = 0,
 	XFS_QM_TRANS_GRP,
+	XFS_QM_TRANS_PRJ,
 	XFS_QM_TRANS_DQTYPES
 };
 #define XFS_QM_TRANS_MAXDQS		2
diff --git a/fs/xfs/xfs_qm_bhv.c b/fs/xfs/xfs_qm_bhv.c
index 2d02eac..437a52d 100644
--- a/fs/xfs/xfs_qm_bhv.c
+++ b/fs/xfs/xfs_qm_bhv.c
@@ -112,16 +112,16 @@ xfs_qm_newmount(
 
 	if (((uquotaondisk && !XFS_IS_UQUOTA_ON(mp)) ||
 	    (!uquotaondisk &&  XFS_IS_UQUOTA_ON(mp)) ||
-	     (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)) ||
+	     (pquotaondisk && !XFS_IS_PQUOTA_ON(mp)) ||
+	    (!pquotaondisk &&  XFS_IS_PQUOTA_ON(mp)))  &&
 	    xfs_dev_is_read_only(mp, "changing quota state")) {
 		xfs_warn(mp, "please mount with%s%s%s%s.",
 			(!quotaondisk ? "out quota" : ""),
 			(uquotaondisk ? " usrquota" : ""),
-			(pquotaondisk ? " prjquota" : ""),
-			(gquotaondisk ? " grpquota" : ""));
+			(gquotaondisk ? " grpquota" : ""),
+			(pquotaondisk ? " prjquota" : ""));
 		return XFS_ERROR(EPERM);
 	}
 
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 09a45af..e4f8b2d 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -119,7 +119,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);
@@ -858,9 +863,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;
 	}
 
@@ -869,10 +876,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 69e3edf..b14f42c 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -288,10 +288,10 @@ typedef struct xfs_qoff_logformat {
  * we didn't have the inode locked, the appropriate dquot(s) will be
  * attached atomically.
  */
-#define XFS_NOT_DQATTACHED(mp, ip) ((XFS_IS_UQUOTA_ON(mp) &&\
-				     (ip)->i_udquot == NULL) || \
-				    (XFS_IS_OQUOTA_ON(mp) && \
-				     (ip)->i_gdquot == 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))
 
 #define XFS_QM_NEED_QUOTACHECK(mp) \
 	((XFS_IS_UQUOTA_ON(mp) && \
@@ -338,17 +338,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 *);
@@ -362,10 +363,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,
+		struct xfs_dquot **pdqp)
 {
 	*udqp = NULL;
 	*gdqp = NULL;
+	*pdqp = NULL;
 	return 0;
 }
 #define xfs_trans_dup_dqinfo(tp, tp2)
@@ -380,14 +383,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)
@@ -401,8 +405,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_symlink.c b/fs/xfs/xfs_symlink.c
index e830fb5..f4895b6 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -360,6 +360,7 @@ xfs_symlink(
 	prid_t			prid;
 	struct xfs_dquot	*udqp = NULL;
 	struct xfs_dquot	*gdqp = NULL;
+	struct xfs_dquot	*pdqp = NULL;
 	uint			resblks;
 
 	*ipp = NULL;
@@ -386,7 +387,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;
 
@@ -427,7 +428,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;
 
@@ -465,7 +467,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);
@@ -563,6 +565,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;
@@ -576,6 +579,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);
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index db041a5..61407a8 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -163,8 +163,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 struct xfs_dqtrx *
@@ -177,8 +179,12 @@ xfs_trans_get_dqtrx(
 
 	if (XFS_QM_ISUDQ(dqp))
 		qa = tp->t_dqinfo->dqs[XFS_QM_TRANS_USR];
-	else
+	else if (XFS_QM_ISGDQ(dqp))
 		qa = tp->t_dqinfo->dqs[XFS_QM_TRANS_GRP];
+	else if (XFS_QM_ISPDQ(dqp))
+		qa = tp->t_dqinfo->dqs[XFS_QM_TRANS_PRJ];
+	else
+		return NULL;
 
 	for (i = 0; i < XFS_QM_TRANS_MAXDQS; i++) {
 		if (qa[i].qt_dquot == NULL ||
@@ -727,8 +733,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.
@@ -743,6 +749,7 @@ xfs_trans_reserve_quota_bydquots(
 	struct xfs_mount	*mp,
 	struct xfs_dquot	*udqp,
 	struct xfs_dquot	*gdqp,
+	struct xfs_dquot	*pdqp,
 	long			nblks,
 	long			ninos,
 	uint			flags)
@@ -770,11 +777,21 @@ xfs_trans_reserve_quota_bydquots(
 			goto unwind_usr;
 	}
 
+	if (pdqp) {
+		error = xfs_trans_dqresv(tp, mp, pdqp, nblks, ninos, flags);
+		if (error)
+			goto unwind_grp;
+	}
+
 	/*
 	 * Didn't change anything critical, so, no need to log
 	 */
 	return 0;
 
+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)
@@ -816,6 +833,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 42c0ef2..dc730ac 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -489,6 +489,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;
@@ -507,7 +508,8 @@ 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;
 
@@ -559,7 +561,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;
 
@@ -623,7 +626,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)
@@ -635,6 +638,7 @@ xfs_create(
 
 	xfs_qm_dqrele(udqp);
 	xfs_qm_dqrele(gdqp);
+	xfs_qm_dqrele(pdqp);
 
 	*ipp = ip;
 	return 0;
@@ -656,6 +660,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);
@@ -1568,7 +1573,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

Re: [PATCH v11 1/4] xfs: Add pquota fields where gquota is used.

[Ben Myers]

On Thu, Jul 11, 2013 at 12:00:40AM -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.
> 
> Not using the pquotino field from superblock yet.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Looks good.  Applied.

Reviewed-by: Ben Myers <bpm@sgi.com>

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

[PATCH v11 2/4] xfs: Start using pquotaino from the superblock.

[Chandra Seetharaman]

Start using pquotino and define a macro to check if the
superblock has pquotino.

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_mount.c             |   62 ++++++++++++++++++++++++++++++++++-----
 fs/xfs/xfs_qm.c                |   28 +++++++++--------
 fs/xfs/xfs_qm_syscalls.c       |   21 +++++++++++++-
 fs/xfs/xfs_sb.h                |    9 +++++-
 fs/xfs/xfs_super.c             |   19 ++++++------
 include/uapi/linux/dqblk_xfs.h |    1 +
 6 files changed, 108 insertions(+), 32 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 2b0ba35..b8a633b 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -336,12 +336,17 @@ xfs_mount_validate_sb(
 		return XFS_ERROR(EWRONGFS);
 	}
 
-	if ((sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) &&
-			(sbp->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.\n");
-		return XFS_ERROR(EFSCORRUPTED);
+	if (xfs_sb_version_has_pquota(sbp)) {
+		if (sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
+			xfs_notice(mp,
+			   "Version 5 of Super block has XFS_OQUOTA bits.\n");
+			return XFS_ERROR(EFSCORRUPTED);
+		}
+	} else if (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
+				XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
+			xfs_notice(mp,
+"Superblock earlier than Version 5 has XFS_[PQ]UOTA_{ENFD|CHKD} bits.\n");
+			return XFS_ERROR(EFSCORRUPTED);
 	}
 
 	/*
@@ -570,8 +575,13 @@ out_unwind:
 }
 
 static void
-xfs_sb_quota_from_disk(struct xfs_sb *sbp)
+xfs_sb_quota_from_disk(struct xfs_mount *mp)
 {
+	struct xfs_sb *sbp = &mp->m_sb;
+
+	if (xfs_sb_version_has_pquota(sbp))
+		return;
+
 	if (sbp->sb_qflags & XFS_OQUOTA_ENFD)
 		sbp->sb_qflags |= (sbp->sb_qflags & XFS_PQUOTA_ACCT) ?
 					XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
@@ -579,6 +589,18 @@ xfs_sb_quota_from_disk(struct xfs_sb *sbp)
 		sbp->sb_qflags |= (sbp->sb_qflags & XFS_PQUOTA_ACCT) ?
 					XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
 	sbp->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
+
+	if (!xfs_sb_version_has_pquota(sbp) && (XFS_IS_PQUOTA_ON(mp))) {
+		/*
+		 * On disk superblock only has sb_gquotino, and incore
+		 * superblock has both sb_gquotino and sb_pquotino.
+		 * But, only one them is supported at any point of time.
+		 * So, if PQUOTA is set in disk superblock, copy over
+		 * sb_gquotino to sb_pquotino.
+		 */
+		mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
+		mp->m_sb.sb_gquotino = NULLFSINO;
+	}
 }
 
 void
@@ -650,6 +672,13 @@ xfs_sb_quota_to_disk(
 {
 	__uint16_t	qflags = from->sb_qflags;
 
+	/*
+	 * We need to do these manipilations only if we are working
+	 * with an older version of on-disk superblock.
+	 */
+	if (xfs_sb_version_has_pquota(from))
+		return;
+
 	if (*fields & XFS_SB_QFLAGS) {
 		/*
 		 * The in-core version of sb_qflags do not have
@@ -669,6 +698,23 @@ xfs_sb_quota_to_disk(
 		to->sb_qflags = cpu_to_be16(qflags);
 		*fields &= ~XFS_SB_QFLAGS;
 	}
+
+	if (*fields & XFS_SB_PQUOTINO && *fields & XFS_SB_GQUOTINO) {
+		/*
+		 * PQUOTINO and GQUOTINO cannot be used together in versions
+		 * of superblock that do not have pquotino. If used, use
+		 * sb_flags to resolve which one should be set.
+		*/
+		if (from->sb_qflags & XFS_PQUOTA_ACCT)
+			*fields &= ~XFS_SB_GQUOTINO;
+		else
+			*fields &= ~XFS_SB_PQUOTINO;
+	}
+
+	if (*fields & XFS_SB_PQUOTINO) {
+		to->sb_gquotino = cpu_to_be64(from->sb_pquotino);
+		*fields &= ~XFS_SB_PQUOTINO;
+	}
 }
 
 /*
@@ -885,7 +931,7 @@ reread:
 	 */
 	xfs_sb_from_disk(&mp->m_sb, XFS_BUF_TO_SBP(bp));
 
-	xfs_sb_quota_from_disk(&mp->m_sb);
+	xfs_sb_quota_from_disk(mp);
 	/*
 	 * We must be able to do sector-sized and sector-aligned IO.
 	 */
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index d320794..ba12dc4 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -860,21 +860,24 @@ xfs_qm_qino_alloc(
 	if (flags & XFS_QMOPT_SBVERSION) {
 		ASSERT(!xfs_sb_version_hasquota(&mp->m_sb));
 		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_VERSIONNUM | XFS_SB_UQUOTINO |
+                        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;
+		/* qflags will get updated fully _after_ quotacheck */
+		mp->m_sb.sb_qflags = mp->m_qflags & XFS_ALL_QUOTA_ACCT;
 	}
 	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);
 
@@ -1484,11 +1487,10 @@ xfs_qm_init_quotainos(
 			if (error)
 				goto error_rele;
 		}
-		/* XXX: Use gquotino for now */
 		if (XFS_IS_PQUOTA_ON(mp) &&
-		    mp->m_sb.sb_gquotino != NULLFSINO) {
-			ASSERT(mp->m_sb.sb_gquotino > 0);
-			error = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino,
+		    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)
 				goto error_rele;
@@ -1496,7 +1498,8 @@ 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);
 	}
 
 	/*
@@ -1524,9 +1527,8 @@ xfs_qm_init_quotainos(
 		flags &= ~XFS_QMOPT_SBVERSION;
 	}
 	if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
-		/* XXX: Use XFS_SB_GQUOTINO for now */
 		error = xfs_qm_qino_alloc(mp, &pip,
-					  sbflags | XFS_SB_GQUOTINO,
+					  sbflags | XFS_SB_PQUOTINO,
 					  flags | XFS_QMOPT_PQUOTA);
 		if (error)
 			goto error_rele;
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index e4f8b2d..132e811 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -296,8 +296,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,8 +415,10 @@ xfs_qm_scall_getqstat(
 	struct xfs_quotainfo	*q = mp->m_quotainfo;
 	struct xfs_inode	*uip = NULL;
 	struct xfs_inode	*gip = NULL;
+	struct xfs_inode	*pip = NULL;
 	bool                    tempuqip = false;
 	bool                    tempgqip = false;
+	bool                    temppqip = false;
 
 	memset(out, 0, sizeof(fs_quota_stat_t));
 
@@ -422,6 +426,7 @@ xfs_qm_scall_getqstat(
 	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 &
@@ -430,10 +435,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,
@@ -445,6 +453,11 @@ xfs_qm_scall_getqstat(
 					0, 0, &gip) == 0)
 			tempgqip = true;
 	}
+	if (!pip && mp->m_sb.sb_pquotino != NULLFSINO) {
+		if (xfs_iget(mp, NULL, mp->m_sb.sb_pquotino,
+					0, 0, &pip) == 0)
+			temppqip = true;
+	}
 	if (uip) {
 		out->qs_uquota.qfs_nblks = uip->i_d.di_nblocks;
 		out->qs_uquota.qfs_nextents = uip->i_d.di_nextents;
@@ -457,6 +470,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_sb.h b/fs/xfs/xfs_sb.h
index 78f9e70..d372fdf 100644
--- a/fs/xfs/xfs_sb.h
+++ b/fs/xfs/xfs_sb.h
@@ -621,7 +621,14 @@ xfs_sb_has_incompat_log_feature(
 static inline bool
 xfs_is_quota_inode(struct xfs_sb *sbp, xfs_ino_t ino)
 {
-	return (ino == sbp->sb_uquotino || ino == sbp->sb_gquotino);
+	return (ino == sbp->sb_uquotino ||
+		ino == sbp->sb_gquotino ||
+		ino == sbp->sb_pquotino);
+}
+
+static inline int xfs_sb_version_has_pquota(xfs_sb_t *sbp)
+{
+	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
 }
 
 /*
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 1d68ffc..5ade637 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -421,12 +421,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;
@@ -556,14 +550,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
@@ -1396,6 +1389,14 @@ xfs_finish_flags(
 		return XFS_ERROR(EROFS);
 	}
 
+	if ((mp->m_qflags & (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE)) &&
+	    (mp->m_qflags & (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE)) &&
+	    !xfs_sb_version_has_pquota(&mp->m_sb)) {
+		xfs_warn(mp,
+		  "Super block does not support project and group quota together");
+		return XFS_ERROR(EINVAL);
+	}
+
 	return 0;
 }
 
diff --git a/include/uapi/linux/dqblk_xfs.h b/include/uapi/linux/dqblk_xfs.h
index 8655280..f17e3bb 100644
--- a/include/uapi/linux/dqblk_xfs.h
+++ b/include/uapi/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

Re: [PATCH v11 2/4] xfs: Start using pquotaino from the superblock.

[Dave Chinner]

On Thu, Jul 11, 2013 at 12:00:41AM -0500, Chandra Seetharaman wrote:
> Start using pquotino and define a macro to check if the
> superblock has pquotino.
> 
> 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_mount.c             |   62 ++++++++++++++++++++++++++++++++++-----
>  fs/xfs/xfs_qm.c                |   28 +++++++++--------
>  fs/xfs/xfs_qm_syscalls.c       |   21 +++++++++++++-
>  fs/xfs/xfs_sb.h                |    9 +++++-
>  fs/xfs/xfs_super.c             |   19 ++++++------
>  include/uapi/linux/dqblk_xfs.h |    1 +
>  6 files changed, 108 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 2b0ba35..b8a633b 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -336,12 +336,17 @@ xfs_mount_validate_sb(
>  		return XFS_ERROR(EWRONGFS);
>  	}
>  
> -	if ((sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) &&
> -			(sbp->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.\n");
> -		return XFS_ERROR(EFSCORRUPTED);
> +	if (xfs_sb_version_has_pquota(sbp)) {
> +		if (sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> +			xfs_notice(mp,
> +			   "Version 5 of Super block has XFS_OQUOTA bits.\n");
> +			return XFS_ERROR(EFSCORRUPTED);
> +		}
> +	} else if (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> +				XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
> +			xfs_notice(mp,
> +"Superblock earlier than Version 5 has XFS_[PQ]UOTA_{ENFD|CHKD} bits.\n");
> +			return XFS_ERROR(EFSCORRUPTED);
>  	}

Can you move this to after we've checked the 5 superblock feature
masks for whether the kernel supports the various features the
superblock defines - feature support checks need to come before
checking superblock fields for validity....

> @@ -570,8 +575,13 @@ out_unwind:
>  }
>  
>  static void
> -xfs_sb_quota_from_disk(struct xfs_sb *sbp)
> +xfs_sb_quota_from_disk(struct xfs_mount *mp)
>  {
> +	struct xfs_sb *sbp = &mp->m_sb;
> +
> +	if (xfs_sb_version_has_pquota(sbp))
> +		return;
> +
>  	if (sbp->sb_qflags & XFS_OQUOTA_ENFD)
>  		sbp->sb_qflags |= (sbp->sb_qflags & XFS_PQUOTA_ACCT) ?
>  					XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
> @@ -579,6 +589,18 @@ xfs_sb_quota_from_disk(struct xfs_sb *sbp)
>  		sbp->sb_qflags |= (sbp->sb_qflags & XFS_PQUOTA_ACCT) ?
>  					XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
>  	sbp->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> +
> +	if (!xfs_sb_version_has_pquota(sbp) && (XFS_IS_PQUOTA_ON(mp))) {
> +		/*
> +		 * On disk superblock only has sb_gquotino, and incore
> +		 * superblock has both sb_gquotino and sb_pquotino.
> +		 * But, only one them is supported at any point of time.
> +		 * So, if PQUOTA is set in disk superblock, copy over
> +		 * sb_gquotino to sb_pquotino.
> +		 */
> +		mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
> +		mp->m_sb.sb_gquotino = NULLFSINO;
> +	}
>  }

Why is this using mp rather than sbp? Indeed, this should be passed
the superblock as a parameter (as per the previous version of the
code) as the superblock we are writing into is determined by the
caller. e.g. what if we want to do this conversion in
xfs_sb_verify() and are using the on-stack struct xfs_sb?

Also, why should a mount option determine how we interpret the
on-disk format? i.e. the XFS_IS_PQUOTA_ON() check? The superblock
itself describes which quota the sb_gquotino contains (i.e. what is
in from->sb_qflags) and that alone determines which field the
on-disk inode gets copied into.

If the mount options then don't line up with what is in the
superblock, then the quota mount checks will handle that
appropriately.

>  
>  void
> @@ -650,6 +672,13 @@ xfs_sb_quota_to_disk(
>  {
>  	__uint16_t	qflags = from->sb_qflags;
>  
> +	/*
> +	 * We need to do these manipilations only if we are working
> +	 * with an older version of on-disk superblock.
> +	 */
> +	if (xfs_sb_version_has_pquota(from))
> +		return;
> +
>  	if (*fields & XFS_SB_QFLAGS) {
>  		/*
>  		 * The in-core version of sb_qflags do not have
> @@ -669,6 +698,23 @@ xfs_sb_quota_to_disk(
>  		to->sb_qflags = cpu_to_be16(qflags);
>  		*fields &= ~XFS_SB_QFLAGS;
>  	}
> +
> +	if (*fields & XFS_SB_PQUOTINO && *fields & XFS_SB_GQUOTINO) {

gcc will throw warnings on that code. The normal way of checking
multiple flag fields is:

	if ((*fields & (XFS_SB_PQUOTINO|XFS_SB_GQUOTINO)) ==
		       (XFS_SB_PQUOTINO|XFS_SB_GQUOTINO)) {

> +		/*
> +		 * PQUOTINO and GQUOTINO cannot be used together in versions
> +		 * of superblock that do not have pquotino. If used, use
> +		 * sb_flags to resolve which one should be set.
> +		*/
> +		if (from->sb_qflags & XFS_PQUOTA_ACCT)
> +			*fields &= ~XFS_SB_GQUOTINO;
> +		else
> +			*fields &= ~XFS_SB_PQUOTINO;
> +	}
> +
> +	if (*fields & XFS_SB_PQUOTINO) {
> +		to->sb_gquotino = cpu_to_be64(from->sb_pquotino);
> +		*fields &= ~XFS_SB_PQUOTINO;
> +	}

Better yet:

	/*
	 * GQUOTINO and PQUOTINO cannot be used together in versions
	 * of superblock that do not have pquotino. from->sb_flags
	 * tells us which quota is active and should be copied to
	 * disk.
	 */
	if ((*fields & XFS_SB_GQUOTINO) && (from->sb_qflags & XFS_GQUOTA_ACCT))
		to->sb_gquotino = cpu_to_be64(from->sb_gquotino);
	else if ((*fields & XFS_SB_PQUOTINO) && (from->sb_qflags & XFS_PQUOTA_ACCT))
		to->sb_gquotino = cpu_to_be64(from->sb_pquotino);

	*fields &= ~(XFS_SB_PQUOTINO | XFS_SB_GQUOTINO);

>  }
>  
>  /*
> @@ -885,7 +931,7 @@ reread:
>  	 */
>  	xfs_sb_from_disk(&mp->m_sb, XFS_BUF_TO_SBP(bp));
>  
> -	xfs_sb_quota_from_disk(&mp->m_sb);
> +	xfs_sb_quota_from_disk(mp);

As I commented above, we need to pass the superblock separately.

> diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
> index 78f9e70..d372fdf 100644
> --- a/fs/xfs/xfs_sb.h
> +++ b/fs/xfs/xfs_sb.h
> @@ -621,7 +621,14 @@ xfs_sb_has_incompat_log_feature(
>  static inline bool
>  xfs_is_quota_inode(struct xfs_sb *sbp, xfs_ino_t ino)
>  {
> -	return (ino == sbp->sb_uquotino || ino == sbp->sb_gquotino);
> +	return (ino == sbp->sb_uquotino ||
> +		ino == sbp->sb_gquotino ||
> +		ino == sbp->sb_pquotino);
> +}

Can you move this function down below the comment in xfs_sb.h that
says:

/*
 * end of superblock version macros
 */

> +
> +static inline int xfs_sb_version_has_pquota(xfs_sb_t *sbp)
> +{
> +	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
>  }

And leave this above it?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH v11 2/4] xfs: Start using pquotaino from the superblock.

[Ben Myers]

Hey Chandra,

On Thu, Jul 11, 2013 at 12:00:41AM -0500, Chandra Seetharaman wrote:
> Start using pquotino and define a macro to check if the
> superblock has pquotino.
> 
> Keep backward compatibilty by alowing mount of older superblock
> with no separate pquota inode.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Here are a few notes from my review.

> ---
>  fs/xfs/xfs_mount.c             |   62 ++++++++++++++++++++++++++++++++++-----
>  fs/xfs/xfs_qm.c                |   28 +++++++++--------
>  fs/xfs/xfs_qm_syscalls.c       |   21 +++++++++++++-
>  fs/xfs/xfs_sb.h                |    9 +++++-
>  fs/xfs/xfs_super.c             |   19 ++++++------
>  include/uapi/linux/dqblk_xfs.h |    1 +
>  6 files changed, 108 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 2b0ba35..b8a633b 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -336,12 +336,17 @@ xfs_mount_validate_sb(
>  		return XFS_ERROR(EWRONGFS);
>  	}
>  
> -	if ((sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) &&
> -			(sbp->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.\n");
> -		return XFS_ERROR(EFSCORRUPTED);
> +	if (xfs_sb_version_has_pquota(sbp)) {

xfs_sb_version_has_pquotino might be a better name for this.

> +		if (sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> +			xfs_notice(mp,
> +			   "Version 5 of Super block has XFS_OQUOTA bits.\n");
> +			return XFS_ERROR(EFSCORRUPTED);
> +		}

If I'm reading that right... Now if you had a v5 super block with oquota
bits it will become unmountable.  Could that happen if you were using
crcs and quotas with a 3.10 kernel and then upgrade?

> +	} else if (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> +				XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
> +			xfs_notice(mp,
> +"Superblock earlier than Version 5 has XFS_[PQ]UOTA_{ENFD|CHKD} bits.\n");
> +			return XFS_ERROR(EFSCORRUPTED);
>  	}
>  
>  	/*
> @@ -570,8 +575,13 @@ out_unwind:
>  }
>  
>  static void
> -xfs_sb_quota_from_disk(struct xfs_sb *sbp)
> +xfs_sb_quota_from_disk(struct xfs_mount *mp)
>  {
> +	struct xfs_sb *sbp = &mp->m_sb;
> +
> +	if (xfs_sb_version_has_pquota(sbp))
> +		return;
> +
>  	if (sbp->sb_qflags & XFS_OQUOTA_ENFD)
>  		sbp->sb_qflags |= (sbp->sb_qflags & XFS_PQUOTA_ACCT) ?
>  					XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
> @@ -579,6 +589,18 @@ xfs_sb_quota_from_disk(struct xfs_sb *sbp)
>  		sbp->sb_qflags |= (sbp->sb_qflags & XFS_PQUOTA_ACCT) ?
>  					XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
>  	sbp->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> +
> +	if (!xfs_sb_version_has_pquota(sbp) && (XFS_IS_PQUOTA_ON(mp))) {
	    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I think you already checked for that up above.

> +		/*
> +		 * On disk superblock only has sb_gquotino, and incore
> +		 * superblock has both sb_gquotino and sb_pquotino.
> +		 * But, only one them is supported at any point of time.
			only one of them 

> +		 * So, if PQUOTA is set in disk superblock, copy over
> +		 * sb_gquotino to sb_pquotino.
> +		 */
> +		mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
> +		mp->m_sb.sb_gquotino = NULLFSINO;
> +	}
>  }
>  
>  void
> @@ -650,6 +672,13 @@ xfs_sb_quota_to_disk(
>  {
>  	__uint16_t	qflags = from->sb_qflags;
>  
> +	/*
> +	 * We need to do these manipilations only if we are working
> +	 * with an older version of on-disk superblock.
> +	 */
> +	if (xfs_sb_version_has_pquota(from))
> +		return;
> +
>  	if (*fields & XFS_SB_QFLAGS) {
>  		/*
>  		 * The in-core version of sb_qflags do not have
> @@ -669,6 +698,23 @@ xfs_sb_quota_to_disk(
>  		to->sb_qflags = cpu_to_be16(qflags);
>  		*fields &= ~XFS_SB_QFLAGS;
>  	}
> +
> +	if (*fields & XFS_SB_PQUOTINO && *fields & XFS_SB_GQUOTINO) {
> +		/*
> +		 * PQUOTINO and GQUOTINO cannot be used together in versions
> +		 * of superblock that do not have pquotino. If used, use
> +		 * sb_flags to resolve which one should be set.
> +		*/
> +		if (from->sb_qflags & XFS_PQUOTA_ACCT)
> +			*fields &= ~XFS_SB_GQUOTINO;
> +		else
> +			*fields &= ~XFS_SB_PQUOTINO;
> +	}
> +
> +	if (*fields & XFS_SB_PQUOTINO) {
> +		to->sb_gquotino = cpu_to_be64(from->sb_pquotino);
> +		*fields &= ~XFS_SB_PQUOTINO;
> +	}
>  }
>  
>  /*
> @@ -885,7 +931,7 @@ reread:
>  	 */
>  	xfs_sb_from_disk(&mp->m_sb, XFS_BUF_TO_SBP(bp));
>  
> -	xfs_sb_quota_from_disk(&mp->m_sb);
> +	xfs_sb_quota_from_disk(mp);
>  	/*
>  	 * We must be able to do sector-sized and sector-aligned IO.
>  	 */
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index d320794..ba12dc4 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -860,21 +860,24 @@ xfs_qm_qino_alloc(
>  	if (flags & XFS_QMOPT_SBVERSION) {
>  		ASSERT(!xfs_sb_version_hasquota(&mp->m_sb));
>  		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_VERSIONNUM | XFS_SB_UQUOTINO |
> +                        XFS_SB_GQUOTINO | XFS_SB_PQUOTINO | XFS_SB_QFLAGS));
	^^^^^^^^^^^^^^^^
Looks like some unwanted spaces in there.

>  		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;
> +		/* qflags will get updated fully _after_ quotacheck */
> +		mp->m_sb.sb_qflags = mp->m_qflags & XFS_ALL_QUOTA_ACCT;

I didn't quite catch why you changed from 0 to ALL_QUOTA_ACCT here, will
take another look.

>  	}
>  	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);
>  
> @@ -1484,11 +1487,10 @@ xfs_qm_init_quotainos(
>  			if (error)
>  				goto error_rele;
>  		}
> -		/* XXX: Use gquotino for now */
>  		if (XFS_IS_PQUOTA_ON(mp) &&
> -		    mp->m_sb.sb_gquotino != NULLFSINO) {
> -			ASSERT(mp->m_sb.sb_gquotino > 0);
> -			error = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino,
> +		    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)
>  				goto error_rele;
> @@ -1496,7 +1498,8 @@ 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);
>  	}
>  
>  	/*
> @@ -1524,9 +1527,8 @@ xfs_qm_init_quotainos(
>  		flags &= ~XFS_QMOPT_SBVERSION;
>  	}
>  	if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
> -		/* XXX: Use XFS_SB_GQUOTINO for now */
>  		error = xfs_qm_qino_alloc(mp, &pip,
> -					  sbflags | XFS_SB_GQUOTINO,
> +					  sbflags | XFS_SB_PQUOTINO,
>  					  flags | XFS_QMOPT_PQUOTA);
>  		if (error)
>  			goto error_rele;
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index e4f8b2d..132e811 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -296,8 +296,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,8 +415,10 @@ xfs_qm_scall_getqstat(

As we discussed on the call:  Now that you've decided to write a new
interface for this, you can be really light touch here in
xfs_qm_scall_getqstat().  The new macro in dqblk_xfs.h can also go away.

Looking good.

Regards,
Ben

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

Re: [PATCH v11 2/4] xfs: Start using pquotaino from the superblock.

[Chandra Seetharaman]

On Thu, 2013-07-11 at 16:16 -0500, Ben Myers wrote:
> Hey Chandra,
> 
> On Thu, Jul 11, 2013 at 12:00:41AM -0500, Chandra Seetharaman wrote:
> > Start using pquotino and define a macro to check if the
> > superblock has pquotino.
> > 
> > Keep backward compatibilty by alowing mount of older superblock
> > with no separate pquota inode.
> > 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> 
> Here are a few notes from my review.
> 
> > ---
> >  fs/xfs/xfs_mount.c             |   62 ++++++++++++++++++++++++++++++++++-----
> >  fs/xfs/xfs_qm.c                |   28 +++++++++--------
> >  fs/xfs/xfs_qm_syscalls.c       |   21 +++++++++++++-
> >  fs/xfs/xfs_sb.h                |    9 +++++-
> >  fs/xfs/xfs_super.c             |   19 ++++++------
> >  include/uapi/linux/dqblk_xfs.h |    1 +
> >  6 files changed, 108 insertions(+), 32 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index 2b0ba35..b8a633b 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -336,12 +336,17 @@ xfs_mount_validate_sb(
> >  		return XFS_ERROR(EWRONGFS);
> >  	}
> >  
> > -	if ((sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) &&
> > -			(sbp->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.\n");
> > -		return XFS_ERROR(EFSCORRUPTED);
> > +	if (xfs_sb_version_has_pquota(sbp)) {
> 
> xfs_sb_version_has_pquotino might be a better name for this.

will fix
> 
> > +		if (sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> > +			xfs_notice(mp,
> > +			   "Version 5 of Super block has XFS_OQUOTA bits.\n");
> > +			return XFS_ERROR(EFSCORRUPTED);
> > +		}
> 
> If I'm reading that right... Now if you had a v5 super block with oquota
> bits it will become unmountable.  Could that happen if you were using
> crcs and quotas with a 3.10 kernel and then upgrade?

yes. My thinking is that v5 (with crc) is still experimental and not
supported yet. Not true ?

Previously this was being fixed without failing. But, Dave suggested
that this certainly would be an inconsistent state and hence need to be
failed. I agreed and changed the code.

The problem you stated can be fixed by adding functionality to
xfs_repair. 
> 
> > +	} else if (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> > +				XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
> > +			xfs_notice(mp,
> > +"Superblock earlier than Version 5 has XFS_[PQ]UOTA_{ENFD|CHKD} bits.\n");
> > +			return XFS_ERROR(EFSCORRUPTED);
> >  	}
> >  
> >  	/*
> > @@ -570,8 +575,13 @@ out_unwind:
> >  }
> >  
> >  static void
> > -xfs_sb_quota_from_disk(struct xfs_sb *sbp)
> > +xfs_sb_quota_from_disk(struct xfs_mount *mp)
> >  {
> > +	struct xfs_sb *sbp = &mp->m_sb;
> > +
> > +	if (xfs_sb_version_has_pquota(sbp))
> > +		return;
> > +
> >  	if (sbp->sb_qflags & XFS_OQUOTA_ENFD)
> >  		sbp->sb_qflags |= (sbp->sb_qflags & XFS_PQUOTA_ACCT) ?
> >  					XFS_PQUOTA_ENFD : XFS_GQUOTA_ENFD;
> > @@ -579,6 +589,18 @@ xfs_sb_quota_from_disk(struct xfs_sb *sbp)
> >  		sbp->sb_qflags |= (sbp->sb_qflags & XFS_PQUOTA_ACCT) ?
> >  					XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
> >  	sbp->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> > +
> > +	if (!xfs_sb_version_has_pquota(sbp) && (XFS_IS_PQUOTA_ON(mp))) {
> 	    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> I think you already checked for that up above.

will fix.
> 
> > +		/*
> > +		 * On disk superblock only has sb_gquotino, and incore
> > +		 * superblock has both sb_gquotino and sb_pquotino.
> > +		 * But, only one them is supported at any point of time.
> 			only one of them 
> 
> > +		 * So, if PQUOTA is set in disk superblock, copy over
> > +		 * sb_gquotino to sb_pquotino.
> > +		 */
> > +		mp->m_sb.sb_pquotino = mp->m_sb.sb_gquotino;
> > +		mp->m_sb.sb_gquotino = NULLFSINO;
> > +	}
> >  }
> >  
> >  void
> > @@ -650,6 +672,13 @@ xfs_sb_quota_to_disk(
> >  {
> >  	__uint16_t	qflags = from->sb_qflags;
> >  
> > +	/*
> > +	 * We need to do these manipilations only if we are working
> > +	 * with an older version of on-disk superblock.
> > +	 */
> > +	if (xfs_sb_version_has_pquota(from))
> > +		return;
> > +
> >  	if (*fields & XFS_SB_QFLAGS) {
> >  		/*
> >  		 * The in-core version of sb_qflags do not have
> > @@ -669,6 +698,23 @@ xfs_sb_quota_to_disk(
> >  		to->sb_qflags = cpu_to_be16(qflags);
> >  		*fields &= ~XFS_SB_QFLAGS;
> >  	}
> > +
> > +	if (*fields & XFS_SB_PQUOTINO && *fields & XFS_SB_GQUOTINO) {
> > +		/*
> > +		 * PQUOTINO and GQUOTINO cannot be used together in versions
> > +		 * of superblock that do not have pquotino. If used, use
> > +		 * sb_flags to resolve which one should be set.
> > +		*/
> > +		if (from->sb_qflags & XFS_PQUOTA_ACCT)
> > +			*fields &= ~XFS_SB_GQUOTINO;
> > +		else
> > +			*fields &= ~XFS_SB_PQUOTINO;
> > +	}
> > +
> > +	if (*fields & XFS_SB_PQUOTINO) {
> > +		to->sb_gquotino = cpu_to_be64(from->sb_pquotino);
> > +		*fields &= ~XFS_SB_PQUOTINO;
> > +	}
> >  }
> >  
> >  /*
> > @@ -885,7 +931,7 @@ reread:
> >  	 */
> >  	xfs_sb_from_disk(&mp->m_sb, XFS_BUF_TO_SBP(bp));
> >  
> > -	xfs_sb_quota_from_disk(&mp->m_sb);
> > +	xfs_sb_quota_from_disk(mp);
> >  	/*
> >  	 * We must be able to do sector-sized and sector-aligned IO.
> >  	 */
> > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > index d320794..ba12dc4 100644
> > --- a/fs/xfs/xfs_qm.c
> > +++ b/fs/xfs/xfs_qm.c
> > @@ -860,21 +860,24 @@ xfs_qm_qino_alloc(
> >  	if (flags & XFS_QMOPT_SBVERSION) {
> >  		ASSERT(!xfs_sb_version_hasquota(&mp->m_sb));
> >  		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_VERSIONNUM | XFS_SB_UQUOTINO |
> > +                        XFS_SB_GQUOTINO | XFS_SB_PQUOTINO | XFS_SB_QFLAGS));
> 	^^^^^^^^^^^^^^^^
> Looks like some unwanted spaces in there.
will fix

> 
> >  		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;
> > +		/* qflags will get updated fully _after_ quotacheck */
> > +		mp->m_sb.sb_qflags = mp->m_qflags & XFS_ALL_QUOTA_ACCT;
> 
> I didn't quite catch why you changed from 0 to ALL_QUOTA_ACCT here, will
> take another look.

We wanted to move all (well, almost all :) the logic of managing
PQUOTINO/GQUOTINO in older superblock in just the two functions
xfs_sb_quota_to/from_disk(). In order to do that I had to use sb_qflags
(you can see that in xfs_sb_quota_to_disk()). So, we have to have
sb_qflags valid before we try to write these fields to the disk. Hence
this change.

> 
> >  	}
> >  	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);
> >  
> > @@ -1484,11 +1487,10 @@ xfs_qm_init_quotainos(
> >  			if (error)
> >  				goto error_rele;
> >  		}
> > -		/* XXX: Use gquotino for now */
> >  		if (XFS_IS_PQUOTA_ON(mp) &&
> > -		    mp->m_sb.sb_gquotino != NULLFSINO) {
> > -			ASSERT(mp->m_sb.sb_gquotino > 0);
> > -			error = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino,
> > +		    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)
> >  				goto error_rele;
> > @@ -1496,7 +1498,8 @@ 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);
> >  	}
> >  
> >  	/*
> > @@ -1524,9 +1527,8 @@ xfs_qm_init_quotainos(
> >  		flags &= ~XFS_QMOPT_SBVERSION;
> >  	}
> >  	if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
> > -		/* XXX: Use XFS_SB_GQUOTINO for now */
> >  		error = xfs_qm_qino_alloc(mp, &pip,
> > -					  sbflags | XFS_SB_GQUOTINO,
> > +					  sbflags | XFS_SB_PQUOTINO,
> >  					  flags | XFS_QMOPT_PQUOTA);
> >  		if (error)
> >  			goto error_rele;
> > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> > index e4f8b2d..132e811 100644
> > --- a/fs/xfs/xfs_qm_syscalls.c
> > +++ b/fs/xfs/xfs_qm_syscalls.c
> > @@ -296,8 +296,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,8 +415,10 @@ xfs_qm_scall_getqstat(
> 
> As we discussed on the call:  Now that you've decided to write a new
> interface for this, you can be really light touch here in
> xfs_qm_scall_getqstat().  The new macro in dqblk_xfs.h can also go away.

yes
> 
> Looking good.
> 
> Regards,
> Ben
> 


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

Re: [PATCH v11 2/4] xfs: Start using pquotaino from the superblock.

[Ben Myers]

Hey Chandra,

On Thu, Jul 11, 2013 at 08:09:58PM -0500, Chandra Seetharaman wrote:
> On Thu, 2013-07-11 at 16:16 -0500, Ben Myers wrote:
> > Hey Chandra,
> > 
> > On Thu, Jul 11, 2013 at 12:00:41AM -0500, Chandra Seetharaman wrote:
> > > Start using pquotino and define a macro to check if the
> > > superblock has pquotino.
> > > 
> > > Keep backward compatibilty by alowing mount of older superblock
> > > with no separate pquota inode.
> > > 
> > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > 
> > Here are a few notes from my review.
> > 
> > > +		if (sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> > > +			xfs_notice(mp,
> > > +			   "Version 5 of Super block has XFS_OQUOTA bits.\n");
> > > +			return XFS_ERROR(EFSCORRUPTED);
> > > +		}
> > 
> > If I'm reading that right... Now if you had a v5 super block with oquota
> > bits it will become unmountable.  Could that happen if you were using
> > crcs and quotas with a 3.10 kernel and then upgrade?
> 
> yes. My thinking is that v5 (with crc) is still experimental and not
> supported yet. Not true ?

Right.  v5 superblock is still experimental.

> Previously this was being fixed without failing. But, Dave suggested
> that this certainly would be an inconsistent state and hence need to be
> failed. I agreed and changed the code.

Sounds fine.  I just wanted to make sure we're all aware of this possibility.

> The problem you stated can be fixed by adding functionality to
> xfs_repair. 

Maybe a workaround for those users would be to shut of quotas before upgrading,
      and then turn them back on after?

> > 
> > >  		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;
> > > +		/* qflags will get updated fully _after_ quotacheck */
> > > +		mp->m_sb.sb_qflags = mp->m_qflags & XFS_ALL_QUOTA_ACCT;
> > 
> > I didn't quite catch why you changed from 0 to ALL_QUOTA_ACCT here, will
> > take another look.
> 
> We wanted to move all (well, almost all :) the logic of managing
> PQUOTINO/GQUOTINO in older superblock in just the two functions
> xfs_sb_quota_to/from_disk(). In order to do that I had to use sb_qflags
> (you can see that in xfs_sb_quota_to_disk()). So, we have to have
> sb_qflags valid before we try to write these fields to the disk. Hence
> this change.

Oh, that explains it.  Thanks.

-Ben

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

Re: [PATCH v11 2/4] xfs: Start using pquotaino from the superblock.

[Chandra Seetharaman]

On Thu, 2013-07-11 at 21:17 -0500, Ben Myers wrote:
> Hey Chandra,
> 
> On Thu, Jul 11, 2013 at 08:09:58PM -0500, Chandra Seetharaman wrote:
> > On Thu, 2013-07-11 at 16:16 -0500, Ben Myers wrote:
> > > Hey Chandra,
> > > 
> > > On Thu, Jul 11, 2013 at 12:00:41AM -0500, Chandra Seetharaman wrote:
> > > > Start using pquotino and define a macro to check if the
> > > > superblock has pquotino.
> > > > 
> > > > Keep backward compatibilty by alowing mount of older superblock
> > > > with no separate pquota inode.
> > > > 
> > > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > > 
> > > Here are a few notes from my review.
> > > 
> > > > +		if (sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> > > > +			xfs_notice(mp,
> > > > +			   "Version 5 of Super block has XFS_OQUOTA bits.\n");
> > > > +			return XFS_ERROR(EFSCORRUPTED);
> > > > +		}
> > > 
> > > If I'm reading that right... Now if you had a v5 super block with oquota
> > > bits it will become unmountable.  Could that happen if you were using
> > > crcs and quotas with a 3.10 kernel and then upgrade?
> > 
> > yes. My thinking is that v5 (with crc) is still experimental and not
> > supported yet. Not true ?
> 
> Right.  v5 superblock is still experimental.
> 
> > Previously this was being fixed without failing. But, Dave suggested
> > that this certainly would be an inconsistent state and hence need to be
> > failed. I agreed and changed the code.
> 
> Sounds fine.  I just wanted to make sure we're all aware of this possibility.
> 
> > The problem you stated can be fixed by adding functionality to
> > xfs_repair. 
> 
> Maybe a workaround for those users would be to shut of quotas before upgrading,
>       and then turn them back on after?

That would work too.
> 
> > > 
> > > >  		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;
> > > > +		/* qflags will get updated fully _after_ quotacheck */
> > > > +		mp->m_sb.sb_qflags = mp->m_qflags & XFS_ALL_QUOTA_ACCT;
> > > 
> > > I didn't quite catch why you changed from 0 to ALL_QUOTA_ACCT here, will
> > > take another look.
> > 
> > We wanted to move all (well, almost all :) the logic of managing
> > PQUOTINO/GQUOTINO in older superblock in just the two functions
> > xfs_sb_quota_to/from_disk(). In order to do that I had to use sb_qflags
> > (you can see that in xfs_sb_quota_to_disk()). So, we have to have
> > sb_qflags valid before we try to write these fields to the disk. Hence
> > this change.
> 
> Oh, that explains it.  Thanks.
> 
> -Ben
> 


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

[PATCH v11 3/4] xfs: Add proper versioning support to fs_quota_stat

[Chandra Seetharaman]

Added appropriate pads and code for backward compatibility.

Copied over the old version as it is different from the newer padded
version.

New callers of the system call have to set the version of the data
structure being passed, and kernel will fill as much data as requested.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/gfs2/quota.c                |    3 --
 fs/quota/quota.c               |   67 +++++++++++++++++++++++++++++++++++++--
 fs/xfs/xfs_qm_syscalls.c       |    4 --
 include/uapi/linux/dqblk_xfs.h |   60 +++++++++++++++++++++++++++++++----
 4 files changed, 116 insertions(+), 18 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index c7c840e..ca0dccd 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1443,9 +1443,6 @@ static int gfs2_quota_get_xstate(struct super_block *sb,
 {
 	struct gfs2_sbd *sdp = sb->s_fs_info;
 
-	memset(fqs, 0, sizeof(struct fs_quota_stat));
-	fqs->qs_version = FS_QSTAT_VERSION;
-
 	switch (sdp->sd_args.ar_quota) {
 	case GFS2_QUOTA_ON:
 		fqs->qs_flags |= (FS_QUOTA_UDQ_ENFD | FS_QUOTA_GDQ_ENFD);
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index c7314f1..ac5dd3a 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -204,15 +204,72 @@ static int quota_setxstate(struct super_block *sb, int cmd, void __user *addr)
 	return sb->s_qcop->set_xstate(sb, flags, cmd);
 }
 
-static int quota_getxstate(struct super_block *sb, void __user *addr)
+static int quota_getxstate(struct super_block *sb, void __user *addr,
+			   bool older_version)
 {
 	struct fs_quota_stat fqs;
-	int ret;
+	struct fs_quota_stat_v1 fqs_v1;
+	int ret, size;
+	void *fqsp;
 
 	if (!sb->s_qcop->get_xstate)
 		return -ENOSYS;
+
+	memset(&fqs, 0, sizeof(struct fs_quota_stat));
+
+	if (older_version) {
+		fqs.qs_version = FS_QSTAT_VERSION;
+		size = FS_QSTAT_V1_SIZE;
+	} else {
+		if (copy_from_user(&fqs, addr, 1)) /* just get the version */
+			return -EFAULT;
+
+		/*
+		 * For forward compatibility. Check against all supported
+		 * versions, and if the version provided is not supported
+		 * by the kernel, set to the highest version supported.
+		 * For now only FS_QSTAT_VERSION_2 is supported.
+		 */
+		if (fqs.qs_version != FS_QSTAT_VERSION_2)
+			fqs.qs_version = FS_QSTAT_VERSION_2;
+
+		/*
+		 * size need to be set based on the version, if
+		 * different versions need different size.
+		 * For now only version 2 exists, so just set size
+		 * to FS_QSTAT_V2_SIZE.
+		 */
+		size = FS_QSTAT_V2_SIZE;
+	}
+
 	ret = sb->s_qcop->get_xstate(sb, &fqs);
-	if (!ret && copy_to_user(addr, &fqs, sizeof(fqs)))
+	if (ret)
+		return ret;
+
+	if (fqs.qs_version == FS_QSTAT_VERSION) {
+		fqs_v1.qs_version = fqs.qs_version;
+		fqs_v1.qs_flags = fqs.qs_flags;
+		fqs_v1.qs_pad = 0;
+
+		fqs_v1.qs_uquota.qfs_ino = fqs.qs_uquota.qfs_ino;
+		fqs_v1.qs_uquota.qfs_nblks = fqs.qs_uquota.qfs_nblks;
+		fqs_v1.qs_uquota.qfs_nextents = fqs.qs_uquota.qfs_nextents;
+
+		fqs_v1.qs_gquota.qfs_ino = fqs.qs_gquota.qfs_ino;
+		fqs_v1.qs_gquota.qfs_nblks = fqs.qs_gquota.qfs_nblks;
+		fqs_v1.qs_gquota.qfs_nextents = fqs.qs_gquota.qfs_nextents;
+
+		fqs_v1.qs_incoredqs = fqs.qs_incoredqs;
+		fqs_v1.qs_btimelimit = fqs.qs_btimelimit;
+		fqs_v1.qs_itimelimit = fqs.qs_itimelimit;
+		fqs_v1.qs_rtbtimelimit = fqs.qs_rtbtimelimit;
+		fqs_v1.qs_bwarnlimit = fqs.qs_bwarnlimit;
+		fqs_v1.qs_iwarnlimit = fqs.qs_iwarnlimit;
+		fqsp = &fqs_v1;
+	} else
+		fqsp = &fqs;
+
+	if (copy_to_user(addr, fqsp, size))
 		return -EFAULT;
 	return ret;
 }
@@ -292,7 +349,9 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
 	case Q_XQUOTARM:
 		return quota_setxstate(sb, cmd, addr);
 	case Q_XGETQSTAT:
-		return quota_getxstate(sb, addr);
+		return quota_getxstate(sb, addr, 1);
+	case Q_XGETQSTATV:
+		return quota_getxstate(sb, addr, 0);
 	case Q_XSETQLIM:
 		return quota_setxquota(sb, type, id, addr);
 	case Q_XGETQUOTA:
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 132e811..02411a7 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -420,9 +420,6 @@ xfs_qm_scall_getqstat(
 	bool                    tempgqip = false;
 	bool                    temppqip = 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;
@@ -432,7 +429,6 @@ xfs_qm_scall_getqstat(
 	out->qs_flags = (__uint16_t) xfs_qm_export_flags(mp->m_qflags &
 							(XFS_ALL_QUOTA_ACCT|
 							 XFS_ALL_QUOTA_ENFD));
-	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)
diff --git a/include/uapi/linux/dqblk_xfs.h b/include/uapi/linux/dqblk_xfs.h
index f17e3bb..f02336b 100644
--- a/include/uapi/linux/dqblk_xfs.h
+++ b/include/uapi/linux/dqblk_xfs.h
@@ -38,6 +38,7 @@
 #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
 #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
 #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
+#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
 
 /*
  * fs_disk_quota structure:
@@ -47,6 +48,7 @@
  * 512 bytes.
  */
 #define FS_DQUOT_VERSION	1	/* fs_disk_quota.d_version */
+
 typedef struct fs_disk_quota {
 	__s8		d_version;	/* version of this structure */
 	__s8		d_flags;	/* FS_{USER,PROJ,GROUP}_QUOTA */
@@ -137,31 +139,75 @@ typedef struct fs_disk_quota {
  * Provides a centralized way to get meta information about the quota subsystem.
  * eg. space taken up for user and group quotas, number of dquots currently
  * incore.
+ * With version FS_QSTAT_VERSION, user space can send in an uninitialized
+ * buffer and data will be filled by the kernel.
+ * Starting FS_QSTAT_VERSION_2, to be used with Q_XGETQSTATV, user space
+ * caller should set qs_version to the appropriate version of the
+ * fs_quota_stat data structure they are providing. On return, user
+ * space should check qs_version and use appropriate fields supported by
+ * that version.
  */
 #define FS_QSTAT_VERSION	1	/* fs_quota_stat.qs_version */
+#define FS_QSTAT_VERSION_2	2	/* new field qs_pquota; realignment */
 
 /*
  * Some basic information about 'quota files'.
  */
-typedef struct fs_qfilestat {
+struct fs_qfilestat_v1 {
 	__u64		qfs_ino;	/* inode number */
 	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
 	__u32		qfs_nextents;	/* number of extents */
-} fs_qfilestat_t;
+};
 
-typedef struct fs_quota_stat {
+struct fs_quota_stat_v1 {
 	__s8		qs_version;	/* version number for future changes */
 	__u16		qs_flags;	/* FS_QUOTA_{U,P,G}DQ_{ACCT,ENFD} */
 	__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
+	struct fs_qfilestat_v1	qs_uquota;	/* user quota information */
+	struct fs_qfilestat_v1	qs_gquota;	/* group quota 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 */
-} fs_quota_stat_t;
+};
+
+/*
+ * Some basic information about 'quota files'. Version 2.
+ */
+struct fs_qfilestat {
+	__u64		qfs_ino;	/* inode number */
+	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
+	__u32		qfs_nextents;	/* number of extents */
+	__u32		qfs_pad;	/* pad for 8-byte alignment */
+};
+
+struct fs_quota_stat {
+	__s8			qs_version;	/* version for future changes */
+	__u8			qs_pad1;	/* pad for 16bit alignment */
+	__u16			qs_flags;	/* FS_QUOTA_.* flags */
+	__u32			qs_incoredqs;	/* number of dquots incore */
+	struct fs_qfilestat	qs_uquota;	/* user quota information */
+	struct fs_qfilestat	qs_gquota;	/* group quota information */
+	struct fs_qfilestat	qs_pquota;	/* project quota information */
+	__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 */
+	__u64			qs_pad2[8];	/* for future proofing */
+};
+
+/*
+ * Since Version 1 did not have padding at appropriate places,
+ * a new data structure has been defined for the older version to
+ * provide backward compatibility.
+ * Future extentions of this data structure won't require new
+ * data structure definitions as the current one can be extended
+ * with the logic and padding in place now.
+ */
+#define FS_QSTAT_V1_SIZE	(sizeof(struct fs_quota_stat_v1))
+#define FS_QSTAT_V2_SIZE	(sizeof(struct fs_quota_stat))
 
 #endif	/* _LINUX_DQBLK_XFS_H */
-- 
1.7.1

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

Re: [PATCH v11 3/4] xfs: Add proper versioning support to fs_quota_stat

[Dave Chinner]

On Thu, Jul 11, 2013 at 12:00:42AM -0500, Chandra Seetharaman wrote:
> Added appropriate pads and code for backward compatibility.
> 
> Copied over the old version as it is different from the newer padded
> version.
> 
> New callers of the system call have to set the version of the data
> structure being passed, and kernel will fill as much data as requested.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> ---
>  fs/gfs2/quota.c                |    3 --
>  fs/quota/quota.c               |   67 +++++++++++++++++++++++++++++++++++++--
>  fs/xfs/xfs_qm_syscalls.c       |    4 --
>  include/uapi/linux/dqblk_xfs.h |   60 +++++++++++++++++++++++++++++++----
>  4 files changed, 116 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index c7c840e..ca0dccd 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -1443,9 +1443,6 @@ static int gfs2_quota_get_xstate(struct super_block *sb,
>  {
>  	struct gfs2_sbd *sdp = sb->s_fs_info;
>  
> -	memset(fqs, 0, sizeof(struct fs_quota_stat));
> -	fqs->qs_version = FS_QSTAT_VERSION;
> -
>  	switch (sdp->sd_args.ar_quota) {
>  	case GFS2_QUOTA_ON:
>  		fqs->qs_flags |= (FS_QUOTA_UDQ_ENFD | FS_QUOTA_GDQ_ENFD);
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index c7314f1..ac5dd3a 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -204,15 +204,72 @@ static int quota_setxstate(struct super_block *sb, int cmd, void __user *addr)
>  	return sb->s_qcop->set_xstate(sb, flags, cmd);
>  }
>  
> -static int quota_getxstate(struct super_block *sb, void __user *addr)
> +static int quota_getxstate(struct super_block *sb, void __user *addr,
> +			   bool older_version)

I'd suggest that the two API calls should be handled by separate
functions so there are no overlapping, dependent branches in the
code.

>  {
>  	struct fs_quota_stat fqs;
> -	int ret;
> +	struct fs_quota_stat_v1 fqs_v1;
> +	int ret, size;
> +	void *fqsp;
>  
>  	if (!sb->s_qcop->get_xstate)
>  		return -ENOSYS;

I'd also suggest that a sb->s_qcop->get_xstatev vector is added
so that filesystems keep the separate as well.

And that means we don't need to play the fs_quota_stat and
conversion-to-fs_quota_stat_v1 games. i.e. the existing
quota_getxstate() remains and uses the existing struct
fs_quota_stat, and the new Q_XGETQSTATV calls quota_getxstatev() and
uses the newly defined struct fs_quota_statv and ->get_xstatev()
call....

> @@ -137,31 +139,75 @@ typedef struct fs_disk_quota {
>   * Provides a centralized way to get meta information about the quota subsystem.
>   * eg. space taken up for user and group quotas, number of dquots currently
>   * incore.
> + * With version FS_QSTAT_VERSION, user space can send in an uninitialized
> + * buffer and data will be filled by the kernel.
> + * Starting FS_QSTAT_VERSION_2, to be used with Q_XGETQSTATV, user space
> + * caller should set qs_version to the appropriate version of the
> + * fs_quota_stat data structure they are providing. On return, user
> + * space should check qs_version and use appropriate fields supported by
> + * that version.
>   */
>  #define FS_QSTAT_VERSION	1	/* fs_quota_stat.qs_version */
> +#define FS_QSTAT_VERSION_2	2	/* new field qs_pquota; realignment */

As per above, this reasoning and logic is all invalid now because we
aren't trying to maintain compatibility between users of the same
quotactl.

We are using a new quotactl, so we do not need to overload the
existing structure or versions.

> +/*
> + * Some basic information about 'quota files'. Version 2.
> + */
> +struct fs_qfilestat {
> +	__u64		qfs_ino;	/* inode number */
> +	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
> +	__u32		qfs_nextents;	/* number of extents */
> +	__u32		qfs_pad;	/* pad for 8-byte alignment */
> +};
> +
> +struct fs_quota_stat {
> +	__s8			qs_version;	/* version for future changes */
> +	__u8			qs_pad1;	/* pad for 16bit alignment */
> +	__u16			qs_flags;	/* FS_QUOTA_.* flags */
> +	__u32			qs_incoredqs;	/* number of dquots incore */
> +	struct fs_qfilestat	qs_uquota;	/* user quota information */
> +	struct fs_qfilestat	qs_gquota;	/* group quota information */
> +	struct fs_qfilestat	qs_pquota;	/* project quota information */
> +	__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 */
> +	__u64			qs_pad2[8];	/* for future proofing */
> +};

these are what become fs_quota_statv and fs_qfilestatv structures
for the new quotactl.

> +/*
> + * Since Version 1 did not have padding at appropriate places,
> + * a new data structure has been defined for the older version to
> + * provide backward compatibility.
> + * Future extentions of this data structure won't require new
> + * data structure definitions as the current one can be extended
> + * with the logic and padding in place now.
> + */
> +#define FS_QSTAT_V1_SIZE	(sizeof(struct fs_quota_stat_v1))
> +#define FS_QSTAT_V2_SIZE	(sizeof(struct fs_quota_stat))

And these can go away completely.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH v11 3/4] xfs: Add proper versioning support to fs_quota_stat

[Steven Whitehouse]

Hi,

Please keep Abhi cc'd on this thread. Thanks,

Steve.

On Thu, 2013-07-11 at 18:05 +1000, Dave Chinner wrote:
> On Thu, Jul 11, 2013 at 12:00:42AM -0500, Chandra Seetharaman wrote:
> > Added appropriate pads and code for backward compatibility.
> > 
> > Copied over the old version as it is different from the newer padded
> > version.
> > 
> > New callers of the system call have to set the version of the data
> > structure being passed, and kernel will fill as much data as requested.
> > 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > ---
> >  fs/gfs2/quota.c                |    3 --
> >  fs/quota/quota.c               |   67 +++++++++++++++++++++++++++++++++++++--
> >  fs/xfs/xfs_qm_syscalls.c       |    4 --
> >  include/uapi/linux/dqblk_xfs.h |   60 +++++++++++++++++++++++++++++++----
> >  4 files changed, 116 insertions(+), 18 deletions(-)
> > 
> > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > index c7c840e..ca0dccd 100644
> > --- a/fs/gfs2/quota.c
> > +++ b/fs/gfs2/quota.c
> > @@ -1443,9 +1443,6 @@ static int gfs2_quota_get_xstate(struct super_block *sb,
> >  {
> >  	struct gfs2_sbd *sdp = sb->s_fs_info;
> >  
> > -	memset(fqs, 0, sizeof(struct fs_quota_stat));
> > -	fqs->qs_version = FS_QSTAT_VERSION;
> > -
> >  	switch (sdp->sd_args.ar_quota) {
> >  	case GFS2_QUOTA_ON:
> >  		fqs->qs_flags |= (FS_QUOTA_UDQ_ENFD | FS_QUOTA_GDQ_ENFD);
> > diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> > index c7314f1..ac5dd3a 100644
> > --- a/fs/quota/quota.c
> > +++ b/fs/quota/quota.c
> > @@ -204,15 +204,72 @@ static int quota_setxstate(struct super_block *sb, int cmd, void __user *addr)
> >  	return sb->s_qcop->set_xstate(sb, flags, cmd);
> >  }
> >  
> > -static int quota_getxstate(struct super_block *sb, void __user *addr)
> > +static int quota_getxstate(struct super_block *sb, void __user *addr,
> > +			   bool older_version)
> 
> I'd suggest that the two API calls should be handled by separate
> functions so there are no overlapping, dependent branches in the
> code.
> 
> >  {
> >  	struct fs_quota_stat fqs;
> > -	int ret;
> > +	struct fs_quota_stat_v1 fqs_v1;
> > +	int ret, size;
> > +	void *fqsp;
> >  
> >  	if (!sb->s_qcop->get_xstate)
> >  		return -ENOSYS;
> 
> I'd also suggest that a sb->s_qcop->get_xstatev vector is added
> so that filesystems keep the separate as well.
> 
> And that means we don't need to play the fs_quota_stat and
> conversion-to-fs_quota_stat_v1 games. i.e. the existing
> quota_getxstate() remains and uses the existing struct
> fs_quota_stat, and the new Q_XGETQSTATV calls quota_getxstatev() and
> uses the newly defined struct fs_quota_statv and ->get_xstatev()
> call....
> 
> > @@ -137,31 +139,75 @@ typedef struct fs_disk_quota {
> >   * Provides a centralized way to get meta information about the quota subsystem.
> >   * eg. space taken up for user and group quotas, number of dquots currently
> >   * incore.
> > + * With version FS_QSTAT_VERSION, user space can send in an uninitialized
> > + * buffer and data will be filled by the kernel.
> > + * Starting FS_QSTAT_VERSION_2, to be used with Q_XGETQSTATV, user space
> > + * caller should set qs_version to the appropriate version of the
> > + * fs_quota_stat data structure they are providing. On return, user
> > + * space should check qs_version and use appropriate fields supported by
> > + * that version.
> >   */
> >  #define FS_QSTAT_VERSION	1	/* fs_quota_stat.qs_version */
> > +#define FS_QSTAT_VERSION_2	2	/* new field qs_pquota; realignment */
> 
> As per above, this reasoning and logic is all invalid now because we
> aren't trying to maintain compatibility between users of the same
> quotactl.
> 
> We are using a new quotactl, so we do not need to overload the
> existing structure or versions.
> 
> > +/*
> > + * Some basic information about 'quota files'. Version 2.
> > + */
> > +struct fs_qfilestat {
> > +	__u64		qfs_ino;	/* inode number */
> > +	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
> > +	__u32		qfs_nextents;	/* number of extents */
> > +	__u32		qfs_pad;	/* pad for 8-byte alignment */
> > +};
> > +
> > +struct fs_quota_stat {
> > +	__s8			qs_version;	/* version for future changes */
> > +	__u8			qs_pad1;	/* pad for 16bit alignment */
> > +	__u16			qs_flags;	/* FS_QUOTA_.* flags */
> > +	__u32			qs_incoredqs;	/* number of dquots incore */
> > +	struct fs_qfilestat	qs_uquota;	/* user quota information */
> > +	struct fs_qfilestat	qs_gquota;	/* group quota information */
> > +	struct fs_qfilestat	qs_pquota;	/* project quota information */
> > +	__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 */
> > +	__u64			qs_pad2[8];	/* for future proofing */
> > +};
> 
> these are what become fs_quota_statv and fs_qfilestatv structures
> for the new quotactl.
> 
> > +/*
> > + * Since Version 1 did not have padding at appropriate places,
> > + * a new data structure has been defined for the older version to
> > + * provide backward compatibility.
> > + * Future extentions of this data structure won't require new
> > + * data structure definitions as the current one can be extended
> > + * with the logic and padding in place now.
> > + */
> > +#define FS_QSTAT_V1_SIZE	(sizeof(struct fs_quota_stat_v1))
> > +#define FS_QSTAT_V2_SIZE	(sizeof(struct fs_quota_stat))
> 
> And these can go away completely.
> 
> Cheers,
> 
> Dave.


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

Re: [PATCH v11 3/4] xfs: Add proper versioning support to fs_quota_stat

[Dave Chinner]

On Thu, Jul 11, 2013 at 12:00:42AM -0500, Chandra Seetharaman wrote:
> Added appropriate pads and code for backward compatibility.
> 
> Copied over the old version as it is different from the newer padded
> version.
> 
> New callers of the system call have to set the version of the data
> structure being passed, and kernel will fill as much data as requested.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Oh, and I forgot to mention:

> +		/*
> +		 * For forward compatibility. Check against all supported
> +		 * versions, and if the version provided is not supported
> +		 * by the kernel, set to the highest version supported.
> +		 * For now only FS_QSTAT_VERSION_2 is supported.
> +		 */
> +		if (fqs.qs_version != FS_QSTAT_VERSION_2)
> +			fqs.qs_version = FS_QSTAT_VERSION_2;

New API, new version number and enforcement: if the version number
passed in from userspace  is not understood, -EINVAL or an
equivalent "parameter not understood" error should be returned.
If the kernel doesn't understand what it is being passed, then
there's no guarantee that anything quota info we return can be
understood by the caller.

And while I remember, we'll also need quotactl(2) man page updates
for the new API.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

[PATCH v11 4/4] xfs: Use new qs_pquota field in fs_quota_stat for Q_XGETQSTATV

[Chandra Seetharaman]

For the Q_XGETQSTATV quota command, if the new version of fs_quota_stat
data structure is used, fill the project quota information.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/xfs_qm_syscalls.c |   38 ++++++++++++++++++++++++++------------
 1 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 02411a7..2154129 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -406,6 +406,14 @@ xfs_qm_scall_quotaon(
 
 /*
  * Return quota status information, such as uquota-off, enforcements, etc.
+ *
+ * We support two different versions of the fs_quota_stat structure
+ * here. If the version is FS_QSTAT_VERSION, then there is no
+ * separate project quota fields, and so we write project quota
+ * information into the group quota fields. With FS_QSTAT_VERSION_2,
+ * there is a separate project quota field so we can use that
+ * instead to pass both group and project quota back to user space.
+ *
  */
 int
 xfs_qm_scall_getqstat(
@@ -420,19 +428,15 @@ xfs_qm_scall_getqstat(
 	bool                    tempgqip = false;
 	bool                    temppqip = false;
 
-	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_uquota.qfs_ino = NULLFSINO;
+	out->qs_gquota.qfs_ino = NULLFSINO;
+	out->qs_pquota.qfs_ino = NULLFSINO;
+	if (!xfs_sb_version_hasquota(&mp->m_sb))
+		return 0;
+
 	out->qs_flags = (__uint16_t) xfs_qm_export_flags(mp->m_qflags &
 							(XFS_ALL_QUOTA_ACCT|
 							 XFS_ALL_QUOTA_ENFD));
-	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;
@@ -454,21 +458,31 @@ xfs_qm_scall_getqstat(
 					0, 0, &pip) == 0)
 			temppqip = true;
 	}
+
 	if (uip) {
+		out->qs_uquota.qfs_ino = mp->m_sb.sb_uquotino;
 		out->qs_uquota.qfs_nblks = uip->i_d.di_nblocks;
 		out->qs_uquota.qfs_nextents = uip->i_d.di_nextents;
 		if (tempuqip)
 			IRELE(uip);
 	}
 	if (gip) {
+		out->qs_gquota.qfs_ino = mp->m_sb.sb_gquotino;
 		out->qs_gquota.qfs_nblks = gip->i_d.di_nblocks;
 		out->qs_gquota.qfs_nextents = gip->i_d.di_nextents;
 		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 (out->qs_version >= FS_QSTAT_VERSION_2) {
+			out->qs_pquota.qfs_ino = mp->m_sb.sb_pquotino;
+			out->qs_pquota.qfs_nblks = pip->i_d.di_nblocks;
+			out->qs_pquota.qfs_nextents = pip->i_d.di_nextents;
+		} else {
+			out->qs_gquota.qfs_ino = mp->m_sb.sb_pquotino;
+			out->qs_gquota.qfs_nblks = pip->i_d.di_nblocks;
+			out->qs_gquota.qfs_nextents = pip->i_d.di_nextents;
+		}
 		if (temppqip)
 			IRELE(pip);
 	}
-- 
1.7.1

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