[PATCH v2 0/6] xfs: some bugfixes and code cleanups for quota

[PATCH v2 0/6] xfs: some bugfixes and code cleanups for quota

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

Hi all,

This patchset include some bugfixes and code cleanups for
qupta.

Changes for v2:
 - put all the scattered patches into a single patchset.
 - update the git version to fix the no function show problem.

Kaixu Xia (6):
  xfs: trace quota allocations for all quota types
  xfs: combine two if statements with same condition
  xfs: reserve quota inode transaction space only when needed
  xfs: remove unnecessary variable udqp from xfs_ioctl_setattr
  xfs: remove unnecessary assertion from xfs_qm_vop_create_dqattach
  xfs: simplify the flags setting in xfs_qm_scall_quotaon

 fs/xfs/xfs_ioctl.c       | 7 ++-----
 fs/xfs/xfs_iops.c        | 5 -----
 fs/xfs/xfs_qm.c          | 7 +++----
 fs/xfs/xfs_qm_syscalls.c | 6 +++---
 4 files changed, 8 insertions(+), 17 deletions(-)

-- 
2.20.0

[PATCH v2 1/6] xfs: trace quota allocations for all quota types

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

The trace event xfs_dquot_dqalloc does not depend on the
value uq, so remove the condition, and trace quota allocations
for all quota types.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 fs/xfs/xfs_qm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index c225691fad15..6678baab37de 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1730,8 +1730,7 @@ xfs_qm_vop_dqalloc(
 			pq = xfs_qm_dqhold(ip->i_pdquot);
 		}
 	}
-	if (uq)
-		trace_xfs_dquot_dqalloc(ip);
+	trace_xfs_dquot_dqalloc(ip);
 
 	xfs_iunlock(ip, lockflags);
 	if (O_udqpp)
-- 
2.20.0

[PATCH v2 2/6] xfs: combine two if statements with same condition

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

The two if statements have same condition, and the mask value
does not change in xfs_setattr_nonsize(), so combine them.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_iops.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index f7a99b3bbcf7..e34814590453 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -738,12 +738,7 @@ xfs_setattr_nonsize(
 			if (error)	/* out of quota */
 				goto out_cancel;
 		}
-	}
 
-	/*
-	 * Change file ownership.  Must be the owner or privileged.
-	 */
-	if (mask & (ATTR_UID|ATTR_GID)) {
 		/*
 		 * CAP_FSETID overrides the following restrictions:
 		 *
-- 
2.20.0

[PATCH v2 3/6] xfs: reserve quota inode transaction space only when needed

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

We share an inode between gquota and pquota with the older
superblock that doesn't have separate pquotino, and for the
need_alloc == false case we don't need to call xfs_dir_ialloc()
function, so add the check if reserved free disk blocks is
needed.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/xfs_qm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 6678baab37de..b684b0410a52 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -780,7 +780,8 @@ xfs_qm_qino_alloc(
 	}
 
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_create,
-			XFS_QM_QINOCREATE_SPACE_RES(mp), 0, 0, &tp);
+			need_alloc ? XFS_QM_QINOCREATE_SPACE_RES(mp) : 0,
+			0, 0, &tp);
 	if (error)
 		return error;
 
-- 
2.20.0

[PATCH v2 4/6] xfs: remove unnecessary variable udqp from xfs_ioctl_setattr

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

The initial value of variable udqp is NULL, and we only set the
flag XFS_QMOPT_PQUOTA in xfs_qm_vop_dqalloc() function, so only
the pdqp value is initialized and the udqp value is still NULL.
Since the udqp value is NULL in the rest part of xfs_ioctl_setattr()
function, it is meaningless and do nothing. So remove it from
xfs_ioctl_setattr().

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index cdfb3cd9a25b..8ff91ab221aa 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1513,7 +1513,6 @@ xfs_ioctl_setattr(
 	struct fsxattr		old_fa;
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*tp;
-	struct xfs_dquot	*udqp = NULL;
 	struct xfs_dquot	*pdqp = NULL;
 	struct xfs_dquot	*olddquot = NULL;
 	int			code;
@@ -1536,7 +1535,7 @@ xfs_ioctl_setattr(
 	if (XFS_IS_QUOTA_ON(mp)) {
 		code = xfs_qm_vop_dqalloc(ip, VFS_I(ip)->i_uid,
 				VFS_I(ip)->i_gid, fa->fsx_projid,
-				XFS_QMOPT_PQUOTA, &udqp, NULL, &pdqp);
+				XFS_QMOPT_PQUOTA, NULL, NULL, &pdqp);
 		if (code)
 			return code;
 	}
@@ -1560,7 +1559,7 @@ xfs_ioctl_setattr(
 
 	if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp) &&
 	    ip->i_d.di_projid != fa->fsx_projid) {
-		code = xfs_qm_vop_chown_reserve(tp, ip, udqp, NULL, pdqp,
+		code = xfs_qm_vop_chown_reserve(tp, ip, NULL, NULL, pdqp,
 				capable(CAP_FOWNER) ?  XFS_QMOPT_FORCE_RES : 0);
 		if (code)	/* out of quota */
 			goto error_trans_cancel;
@@ -1626,7 +1625,6 @@ xfs_ioctl_setattr(
 	 * Release any dquot(s) the inode had kept before chown.
 	 */
 	xfs_qm_dqrele(olddquot);
-	xfs_qm_dqrele(udqp);
 	xfs_qm_dqrele(pdqp);
 
 	return code;
@@ -1634,7 +1632,6 @@ xfs_ioctl_setattr(
 error_trans_cancel:
 	xfs_trans_cancel(tp);
 error_free_dquots:
-	xfs_qm_dqrele(udqp);
 	xfs_qm_dqrele(pdqp);
 	return code;
 }
-- 
2.20.0

[PATCH v2 5/6] xfs: remove unnecessary assertion from xfs_qm_vop_create_dqattach

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

The check XFS_IS_QUOTA_RUNNING() has been done when enter the
xfs_qm_vop_create_dqattach() function, it will return directly
if the result is false, so the followed XFS_IS_QUOTA_RUNNING()
assertion is unnecessary. If we truly care about this, the check
also can be added to the condition of next if statements.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
 fs/xfs/xfs_qm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index b684b0410a52..fc93f88a9926 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1932,7 +1932,6 @@ xfs_qm_vop_create_dqattach(
 		return;
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-	ASSERT(XFS_IS_QUOTA_RUNNING(mp));
 
 	if (udqp && XFS_IS_UQUOTA_ON(mp)) {
 		ASSERT(ip->i_udquot == NULL);
-- 
2.20.0

[PATCH v2 6/6] xfs: simplify the flags setting in xfs_qm_scall_quotaon

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

Simplify the setting of the flags value, and only consider
quota enforcement stuff here.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
 fs/xfs/xfs_qm_syscalls.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 5d5ac65aa1cc..944486f2b287 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -357,11 +357,11 @@ xfs_qm_scall_quotaon(
 	int		error;
 	uint		qf;
 
-	flags &= (XFS_ALL_QUOTA_ACCT | XFS_ALL_QUOTA_ENFD);
 	/*
-	 * Switching on quota accounting must be done at mount time.
+	 * Switching on quota accounting must be done at mount time,
+	 * only consider quota enforcement stuff here.
 	 */
-	flags &= ~(XFS_ALL_QUOTA_ACCT);
+	flags &= XFS_ALL_QUOTA_ENFD;
 
 	if (flags == 0) {
 		xfs_debug(mp, "%s: zero flags, m_qflags=%x",
-- 
2.20.0

Re: [PATCH v2 0/6] xfs: some bugfixes and code cleanups for quota

[Darrick J. Wong]

On Sat, Apr 11, 2020 at 05:12:52PM +0800, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> Hi all,
> 
> This patchset include some bugfixes and code cleanups for
> qupta.

The series looks ok to me; thanks for bundling this up. :)

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> 
> Changes for v2:
>  - put all the scattered patches into a single patchset.
>  - update the git version to fix the no function show problem.
> 
> Kaixu Xia (6):
>   xfs: trace quota allocations for all quota types
>   xfs: combine two if statements with same condition
>   xfs: reserve quota inode transaction space only when needed
>   xfs: remove unnecessary variable udqp from xfs_ioctl_setattr
>   xfs: remove unnecessary assertion from xfs_qm_vop_create_dqattach
>   xfs: simplify the flags setting in xfs_qm_scall_quotaon
> 
>  fs/xfs/xfs_ioctl.c       | 7 ++-----
>  fs/xfs/xfs_iops.c        | 5 -----
>  fs/xfs/xfs_qm.c          | 7 +++----
>  fs/xfs/xfs_qm_syscalls.c | 6 +++---
>  4 files changed, 8 insertions(+), 17 deletions(-)
> 
> -- 
> 2.20.0
>