[RFC PATCH 1/4] Define a new function xfs_this_quota_on()

[RFC PATCH 1/4] Define a new function xfs_this_quota_on()

[Chandra Seetharaman]

Create a new function xfs_this_quota_on() that takes a xfs_mount
data srtucture and a disk quota type and returns true if the specified
type of quota is ON in the xfs_mount data structure.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/xfs_dquot.c |    4 ++--
 fs/xfs/xfs_dquot.h |   17 +++++++++++++----
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 25d7280..815e231 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -372,7 +372,7 @@ xfs_qm_dqalloc(
 	 * Return if this type of quotas is turned off while we didn't
 	 * have an inode lock
 	 */
-	if (XFS_IS_THIS_QUOTA_OFF(dqp)) {
+	if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
 		xfs_iunlock(quotip, XFS_ILOCK_EXCL);
 		return (ESRCH);
 	}
@@ -474,7 +474,7 @@ xfs_qm_dqtobp(
 	dqp->q_fileoffset = (xfs_fileoff_t)id / mp->m_quotainfo->qi_dqperchunk;
 
 	xfs_ilock(quotip, XFS_ILOCK_SHARED);
-	if (XFS_IS_THIS_QUOTA_OFF(dqp)) {
+	if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
 		/*
 		 * Return if this type of quotas is turned off while we
 		 * didn't have the quota inode lock.
diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 34b7e94..ef7a312 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -102,6 +102,19 @@ static inline void xfs_dqfunlock(xfs_dquot_t *dqp)
 	complete(&dqp->q_flush);
 }
 
+static inline int xfs_this_quota_on(struct xfs_mount *mp, int type)
+{
+	type &= XFS_DQ_ALLTYPES;
+	switch(type) {
+	case XFS_DQ_USER:
+		return XFS_IS_UQUOTA_ON(mp);
+	case XFS_DQ_GROUP:
+	case XFS_DQ_PROJ:
+	default:
+		return XFS_IS_OQUOTA_ON(mp);
+	}
+}
+
 #define XFS_DQ_IS_LOCKED(dqp)	(mutex_is_locked(&((dqp)->q_qlock)))
 #define XFS_DQ_IS_DIRTY(dqp)	((dqp)->dq_flags & XFS_DQ_DIRTY)
 #define XFS_QM_ISUDQ(dqp)	((dqp)->dq_flags & XFS_DQ_USER)
@@ -112,10 +125,6 @@ static inline void xfs_dqfunlock(xfs_dquot_t *dqp)
 				 XFS_DQ_TO_QINF(dqp)->qi_uquotaip : \
 				 XFS_DQ_TO_QINF(dqp)->qi_gquotaip)
 
-#define XFS_IS_THIS_QUOTA_OFF(d) (! (XFS_QM_ISUDQ(d) ? \
-				     (XFS_IS_UQUOTA_ON((d)->q_mount)) : \
-				     (XFS_IS_OQUOTA_ON((d)->q_mount))))
-
 extern void		xfs_qm_dqdestroy(xfs_dquot_t *);
 extern int		xfs_qm_dqflush(xfs_dquot_t *, uint);
 extern int		xfs_qm_dqpurge(xfs_dquot_t *);
-- 
1.7.1

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

Re: [RFC PATCH 1/4] Define a new function xfs_this_quota_on()

[Christoph Hellwig]

On Thu, Oct 27, 2011 at 05:05:29PM -0500, Chandra Seetharaman wrote:
> Create a new function xfs_this_quota_on() that takes a xfs_mount
> data srtucture and a disk quota type and returns true if the specified
> type of quota is ON in the xfs_mount data structure.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

Re: [RFC PATCH 1/4] Define a new function xfs_this_quota_on()

[Alex Elder]

On Thu, 2011-10-27 at 17:05 -0500, Chandra Seetharaman wrote:
> Create a new function xfs_this_quota_on() that takes a xfs_mount
> data srtucture and a disk quota type and returns true if the specified
> type of quota is ON in the xfs_mount data structure.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

[RFC PATCH 0/4] Rearrange code to make the code more readable

[Chandra Seetharaman]

Hello All,

These define some new inline functions and macros, rearrange
the code for easier reading, simplification of code paths
and prepare for the changes to support pquota and gquota
simultaneously.

This is just a repost of the patch posted on 10/27/2011:
http://marc.info/?l=linux-xfs&m=131975420304313&w=2

Regards,

chandra

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

[RFC PATCH 1/4] Define a new function xfs_this_quota_on()

[Chandra Seetharaman]

Create a new function xfs_this_quota_on() that takes a xfs_mount
data srtucture and a disk quota type and returns true if the specified
type of quota is ON in the xfs_mount data structure.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/xfs_dquot.c |    4 ++--
 fs/xfs/xfs_dquot.h |   18 ++++++++++++++----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index a1d91d8..221b22e 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -115,6 +115,20 @@ static inline void xfs_dqunlock_nonotify(struct xfs_dquot *dqp)
 	mutex_unlock(&dqp->q_qlock);
 }
 
+static inline int xfs_this_quota_on(struct xfs_mount *mp, int type)
+{
+	type &= XFS_DQ_ALLTYPES;
+	switch(type) {
+	case XFS_DQ_USER:
+		return XFS_IS_UQUOTA_ON(mp);
+	case XFS_DQ_GROUP:
+	case XFS_DQ_PROJ:
+		return XFS_IS_OQUOTA_ON(mp);
+	default:
+		return 0;
+	}
+}
+
 #define XFS_DQ_IS_LOCKED(dqp)	(mutex_is_locked(&((dqp)->q_qlock)))
 #define XFS_DQ_IS_DIRTY(dqp)	((dqp)->dq_flags & XFS_DQ_DIRTY)
 #define XFS_QM_ISUDQ(dqp)	((dqp)->dq_flags & XFS_DQ_USER)
@@ -125,10 +139,6 @@ static inline void xfs_dqunlock_nonotify(struct xfs_dquot *dqp)
 				 XFS_DQ_TO_QINF(dqp)->qi_uquotaip : \
 				 XFS_DQ_TO_QINF(dqp)->qi_gquotaip)
 
-#define XFS_IS_THIS_QUOTA_OFF(d) (! (XFS_QM_ISUDQ(d) ? \
-				     (XFS_IS_UQUOTA_ON((d)->q_mount)) : \
-				     (XFS_IS_OQUOTA_ON((d)->q_mount))))
-
 extern int		xfs_qm_dqread(struct xfs_mount *, xfs_dqid_t, uint,
 					uint, struct xfs_dquot	**);
 extern void		xfs_qm_dqdestroy(xfs_dquot_t *);
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index b4ff40b..4c8b3d2 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -358,7 +358,7 @@ xfs_qm_dqalloc(
 	 * Return if this type of quotas is turned off while we didn't
 	 * have an inode lock
 	 */
-	if (XFS_IS_THIS_QUOTA_OFF(dqp)) {
+	if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
 		xfs_iunlock(quotip, XFS_ILOCK_EXCL);
 		return (ESRCH);
 	}
@@ -460,7 +460,7 @@ xfs_qm_dqtobp(
 	dqp->q_fileoffset = (xfs_fileoff_t)id / mp->m_quotainfo->qi_dqperchunk;
 
 	xfs_ilock(quotip, XFS_ILOCK_SHARED);
-	if (XFS_IS_THIS_QUOTA_OFF(dqp)) {
+	if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
 		/*
 		 * Return if this type of quotas is turned off while we
 		 * didn't have the quota inode lock.
-- 
1.7.1

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

[RFC PATCH 2/4] Define a new function xfs_inode_dquot()

[Chandra Seetharaman]

Define a new function xfs_inode_dquot() that takes a inode pointer
and a disk quota type and returns the quota pointer for the specified
quota type.

This simplifies the xfs_qm_dqget() error path significantly.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/xfs_dquot.c |   33 +++++++++------------------------
 fs/xfs/xfs_dquot.h |   14 ++++++++++++++
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 221b22e..41cb5f4 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -129,6 +129,20 @@ static inline int xfs_this_quota_on(struct xfs_mount *mp, int type)
 	}
 }
 
+static inline xfs_dquot_t *xfs_inode_dquot(struct xfs_inode *ip, int type)
+{
+	type &= XFS_DQ_ALLTYPES;
+	switch(type) {
+	case XFS_DQ_USER:
+		return ip->i_udquot;
+	case XFS_DQ_PROJ:
+	case XFS_DQ_GROUP:
+		return ip->i_gdquot;
+	default:
+		return NULL;
+	}
+}
+
 #define XFS_DQ_IS_LOCKED(dqp)	(mutex_is_locked(&((dqp)->q_qlock)))
 #define XFS_DQ_IS_DIRTY(dqp)	((dqp)->dq_flags & XFS_DQ_DIRTY)
 #define XFS_QM_ISUDQ(dqp)	((dqp)->dq_flags & XFS_DQ_USER)
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 4c8b3d2..bf4fe86 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -723,7 +723,7 @@ xfs_qm_dqget(
 	uint		flags,	  /* DQALLOC, DQSUSER, DQREPAIR, DOWARN */
 	xfs_dquot_t	**O_dqpp) /* OUT : locked incore dquot */
 {
-	xfs_dquot_t	*dqp;
+	xfs_dquot_t	*dqp, *dqp1;
 	xfs_dqhash_t	*h;
 	uint		version;
 	int		error;
@@ -750,10 +750,7 @@ xfs_qm_dqget(
 	       type == XFS_DQ_GROUP);
 	if (ip) {
 		ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-		if (type == XFS_DQ_USER)
-			ASSERT(ip->i_udquot == NULL);
-		else
-			ASSERT(ip->i_gdquot == NULL);
+		ASSERT(xfs_inode_dquot(ip, type) == NULL);
 	}
 #endif
 
@@ -819,30 +816,18 @@ restart:
 		 * A dquot could be attached to this inode by now, since
 		 * we had dropped the ilock.
 		 */
-		if (type == XFS_DQ_USER) {
-			if (!XFS_IS_UQUOTA_ON(mp)) {
-				/* inode stays locked on return */
-				xfs_qm_dqdestroy(dqp);
-				return XFS_ERROR(ESRCH);
-			}
-			if (ip->i_udquot) {
+		if (xfs_this_quota_on(mp, type)) {
+			dqp1 = xfs_inode_dquot(ip, type);
+			if (dqp1) {
 				xfs_qm_dqdestroy(dqp);
-				dqp = ip->i_udquot;
+				dqp = dqp1;
 				xfs_dqlock(dqp);
 				goto dqret;
 			}
 		} else {
-			if (!XFS_IS_OQUOTA_ON(mp)) {
-				/* inode stays locked on return */
-				xfs_qm_dqdestroy(dqp);
-				return XFS_ERROR(ESRCH);
-			}
-			if (ip->i_gdquot) {
-				xfs_qm_dqdestroy(dqp);
-				dqp = ip->i_gdquot;
-				xfs_dqlock(dqp);
-				goto dqret;
-			}
+			/* inode stays locked on return */
+			xfs_qm_dqdestroy(dqp);
+			return XFS_ERROR(ESRCH);
 		}
 	}
 
-- 
1.7.1

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

[RFC PATCH 3/4] Change xfs_sb_from_disk() interface to take a mount pointer

[Chandra Seetharaman]

Change xfs_sb_from_disk() interface to take a mount pointer
instead of a superblock pointer.

This is to print mount point specific error messages in future
fixes.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/xfs_log_recover.c |    2 +-
 fs/xfs/xfs_mount.c       |    6 ++++--
 fs/xfs/xfs_mount.h       |    2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 19f69e2..c082e44 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -395,7 +395,7 @@ extern void	xfs_set_low_space_thresholds(struct xfs_mount *);
 extern void	xfs_mod_sb(struct xfs_trans *, __int64_t);
 extern int	xfs_initialize_perag(struct xfs_mount *, xfs_agnumber_t,
 					xfs_agnumber_t *);
-extern void	xfs_sb_from_disk(struct xfs_sb *, struct xfs_dsb *);
+extern void	xfs_sb_from_disk(struct xfs_mount *, struct xfs_dsb *);
 extern void	xfs_sb_to_disk(struct xfs_dsb *, struct xfs_sb *, __int64_t);
 
 #endif	/* __XFS_MOUNT_H__ */
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 541a508..b38eb84 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3695,7 +3695,7 @@ xlog_do_recover(
 
 	/* Convert superblock from on-disk format */
 	sbp = &log->l_mp->m_sb;
-	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
+	xfs_sb_from_disk(log->l_mp, XFS_BUF_TO_SBP(bp));
 	ASSERT(sbp->sb_magicnum == XFS_SB_MAGIC);
 	ASSERT(xfs_sb_good_version(sbp));
 	xfs_buf_relse(bp);
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index d06afbc..25e9908 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -553,9 +553,11 @@ out_unwind:
 
 void
 xfs_sb_from_disk(
-	xfs_sb_t	*to,
+	struct xfs_mount	*mp,
 	xfs_dsb_t	*from)
 {
+	struct xfs_sb *to = &mp->m_sb;
+
 	to->sb_magicnum = be32_to_cpu(from->sb_magicnum);
 	to->sb_blocksize = be32_to_cpu(from->sb_blocksize);
 	to->sb_dblocks = be64_to_cpu(from->sb_dblocks);
@@ -693,7 +695,7 @@ reread:
 	 * Initialize the mount structure from the superblock.
 	 * But first do some basic consistency checking.
 	 */
-	xfs_sb_from_disk(&mp->m_sb, XFS_BUF_TO_SBP(bp));
+	xfs_sb_from_disk(mp, XFS_BUF_TO_SBP(bp));
 	error = xfs_mount_validate_sb(mp, &(mp->m_sb), flags);
 	if (error) {
 		if (loud)
-- 
1.7.1

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

[RFC PATCH 4/4] Define new macro XFS_ALL_QUOTA_ACTIVE and simply some usage

[Chandra Seetharaman]

Define new macro XFS_ALL_QUOTA_ACTIVE and simply some usage
of quota macros.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/xfs_qm.c    |    2 +-
 fs/xfs/xfs_quota.h |    2 ++
 fs/xfs/xfs_super.c |    7 +++----
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
index 8a0807e..b50ec5b 100644
--- a/fs/xfs/xfs_quota.h
+++ b/fs/xfs/xfs_quota.h
@@ -174,6 +174,8 @@ typedef struct xfs_qoff_logformat {
 #define XFS_UQUOTA_ACTIVE	0x0100  /* uquotas are being turned off */
 #define XFS_PQUOTA_ACTIVE	0x0200  /* pquotas are being turned off */
 #define XFS_GQUOTA_ACTIVE	0x0400  /* gquotas are being turned off */
+#define XFS_ALL_QUOTA_ACTIVE	\
+	(XFS_UQUOTA_ACTIVE | XFS_PQUOTA_ACTIVE | XFS_GQUOTA_ACTIVE)
 
 /*
  * Checking XFS_IS_*QUOTA_ON() while holding any inode lock guarantees
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 671f37e..1b2f5b3 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1499,7 +1499,7 @@ xfs_qm_quotacheck(
 	 * quotachecked status, since we won't be doing accounting for
 	 * that type anymore.
 	 */
-	mp->m_qflags &= ~(XFS_OQUOTA_CHKD | XFS_UQUOTA_CHKD);
+	mp->m_qflags &= ~XFS_ALL_QUOTA_CHKD;
 	mp->m_qflags |= flags;
 
  error_return:
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index ee5b695..5e0d43f 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -324,10 +324,9 @@ xfs_parseargs(
 		} else if (!strcmp(this_char, MNTOPT_FILESTREAM)) {
 			mp->m_flags |= XFS_MOUNT_FILESTREAMS;
 		} else if (!strcmp(this_char, MNTOPT_NOQUOTA)) {
-			mp->m_qflags &= ~(XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
-					  XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
-					  XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
-					  XFS_UQUOTA_ENFD | XFS_OQUOTA_ENFD);
+			mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
+			mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
+			mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
 		} else if (!strcmp(this_char, MNTOPT_QUOTA) ||
 			   !strcmp(this_char, MNTOPT_UQUOTA) ||
 			   !strcmp(this_char, MNTOPT_USRQUOTA)) {
-- 
1.7.1

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

Re: [RFC PATCH 1/4] Define a new function xfs_this_quota_on()

[Christoph Hellwig]

On Mon, Jan 23, 2012 at 11:31:25AM -0600, Chandra Seetharaman wrote:
> Create a new function xfs_this_quota_on() that takes a xfs_mount
> data srtucture and a disk quota type and returns true if the specified
> type of quota is ON in the xfs_mount data structure.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Looks good except for a tiny style nitpick.

Reviewed-by: Christoph Hellwig <hch@lst.de>

> +	switch(type) {

This should have a whitespace after the opening brace.

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

Re: [RFC PATCH 2/4] Define a new function xfs_inode_dquot()

[Christoph Hellwig]

On Mon, Jan 23, 2012 at 11:31:30AM -0600, Chandra Seetharaman wrote:
> Define a new function xfs_inode_dquot() that takes a inode pointer
> and a disk quota type and returns the quota pointer for the specified
> quota type.
> 
> This simplifies the xfs_qm_dqget() error path significantly.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

> +static inline xfs_dquot_t *xfs_inode_dquot(struct xfs_inode *ip, int type)
> +{
> +	type &= XFS_DQ_ALLTYPES;
> +	switch(type) {

Same style nitpick as for the last patch.

Btw, instead of masking out XFS_DQ_ALLTYPES first this would be more
readable as:

	switch (type & XFS_DQ_ALLTYPES) {

(that also applies to the previous patch).

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

Re: [RFC PATCH 3/4] Change xfs_sb_from_disk() interface to take a mount pointer

[Christoph Hellwig]

On Mon, Jan 23, 2012 at 11:31:37AM -0600, Chandra Seetharaman wrote:
> Change xfs_sb_from_disk() interface to take a mount pointer
> instead of a superblock pointer.
> 
> This is to print mount point specific error messages in future
> fixes.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

Re: [RFC PATCH 4/4] Define new macro XFS_ALL_QUOTA_ACTIVE and simply some usage

[Christoph Hellwig]

On Mon, Jan 23, 2012 at 11:31:43AM -0600, Chandra Seetharaman wrote:
> Define new macro XFS_ALL_QUOTA_ACTIVE and simply some usage
> of quota macros.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

Re: [RFC PATCH 2/4] Define a new function xfs_inode_dquot()

[Chandra Seetharaman]

On Tue, 2012-01-24 at 12:48 -0500, Christoph Hellwig wrote:
> On Mon, Jan 23, 2012 at 11:31:30AM -0600, Chandra Seetharaman wrote:
> > Define a new function xfs_inode_dquot() that takes a inode pointer
> > and a disk quota type and returns the quota pointer for the specified
> > quota type.
> > 
> > This simplifies the xfs_qm_dqget() error path significantly.
> 
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> > +static inline xfs_dquot_t *xfs_inode_dquot(struct xfs_inode *ip, int type)
> > +{
> > +	type &= XFS_DQ_ALLTYPES;
> > +	switch(type) {
> 
> Same style nitpick as for the last patch.
> 
> Btw, instead of masking out XFS_DQ_ALLTYPES first this would be more
> readable as:
> 
> 	switch (type & XFS_DQ_ALLTYPES) {
> 
> (that also applies to the previous patch).
> 

Will fix both of these in the next version.

Thanks for the review Christoph.

Chandra


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

Re: [RFC PATCH 1/4] Define a new function xfs_this_quota_on()

[Ben Myers]

On Tue, Jan 24, 2012 at 12:47:12PM -0500, Christoph Hellwig wrote:
> On Mon, Jan 23, 2012 at 11:31:25AM -0600, Chandra Seetharaman wrote:
> > Create a new function xfs_this_quota_on() that takes a xfs_mount
> > data srtucture and a disk quota type and returns true if the specified
> > type of quota is ON in the xfs_mount data structure.
> > 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> 
> Looks good except for a tiny style nitpick.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> > +	switch(type) {
> 
> This should have a whitespace after the opening brace.

I've added the space.

This patch looks good.

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

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

Re: [RFC PATCH 2/4] Define a new function xfs_inode_dquot()

[Ben Myers]

Hey Chandra,

On Tue, Jan 24, 2012 at 01:41:36PM -0600, Chandra Seetharaman wrote:
> On Tue, 2012-01-24 at 12:48 -0500, Christoph Hellwig wrote:
> > On Mon, Jan 23, 2012 at 11:31:30AM -0600, Chandra Seetharaman wrote:
> > > Define a new function xfs_inode_dquot() that takes a inode pointer
> > > and a disk quota type and returns the quota pointer for the specified
> > > quota type.
> > > 
> > > This simplifies the xfs_qm_dqget() error path significantly.
> > 
> > Looks good,
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> >
> > > +static inline xfs_dquot_t *xfs_inode_dquot(struct xfs_inode *ip, int type)
> > > +{
> > > +	type &= XFS_DQ_ALLTYPES;
> > > +	switch(type) {
> > 
> > Same style nitpick as for the last patch.
> > 
> > Btw, instead of masking out XFS_DQ_ALLTYPES first this would be more
> > readable as:
> > 
> > 	switch (type & XFS_DQ_ALLTYPES) {
> > 
> > (that also applies to the previous patch).
> > 
> 
> Will fix both of these in the next version.

I'll fix them up.  No need to post again.  I also twiddled the order of
USER/GROUP/PROJ in xfs_inode_dquot to match xfs_this_quota_on.

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

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

Re: [RFC PATCH 3/4] Change xfs_sb_from_disk() interface to take a mount pointer

[Ben Myers]

On Mon, Jan 23, 2012 at 11:31:37AM -0600, Chandra Seetharaman wrote:
> Change xfs_sb_from_disk() interface to take a mount pointer
> instead of a superblock pointer.
> 
> This is to print mount point specific error messages in future
> fixes.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

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

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

Re: [RFC PATCH 4/4] Define new macro XFS_ALL_QUOTA_ACTIVE and simply some usage

[Ben Myers]

On Mon, Jan 23, 2012 at 11:31:43AM -0600, Chandra Seetharaman wrote:
> Define new macro XFS_ALL_QUOTA_ACTIVE and simply some usage
> of quota macros.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

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

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