[PATCH 0/2] xfs: fix project quota ENOSPC vs EDQUOT

[PATCH 0/2] xfs: fix project quota ENOSPC vs EDQUOT

[Eric Sandeen]

Currently when project quota is enabled, group quota will return
ENOSPC vs EDQUOT.  This is the right error code for project quota
but not for group quota; this is an old bug left over from when they
were mutually exclusive.

[PATCH 1/2] xfs: group quota should return EDQUOT when prj quota enabled

[Eric Sandeen]

Long ago, group & project quota were mutually exclusive, and so
when we turned on XFS_QMOPT_ENOSPC ("return ENOSPC if project quota
is exceeded") when project quota was enabled, we only needed to
disable it again for user quota.

When group & project quota got separated, this got missed, and as a
result if project quota is enabled and group quota is exceeded, the
error code returned is incorrectly returned as ENOSPC not EDQUOT.

Fix this by stripping XFS_QMOPT_ENOSPC out of flags for group
quota when we try to reserve the space.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index e4eca607e512..47c6e88c9db6 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -757,7 +757,8 @@ xfs_trans_reserve_quota_bydquots(
 	}
 
 	if (gdqp) {
-		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags);
+		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos,
+					(flags & ~XFS_QMOPT_ENOSPC));
 		if (error)
 			goto unwind_usr;
 	}

[PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Eric Sandeen]

The only place we return -EDQUOT, and therefore need to make a decision
about returning -ENOSPC for project quota instead, is in xfs_trans_dqresv().

So there's no reason to be setting and clearing XFS_QMOPT_ENOSPC at higher
levels; if xfs_trans_dqresv has failed, test if the dqp we were were handed
is a project quota and if so, return -ENOSPC instead of EDQUOT.  The
complexity is just a leftover from when project & group quota were mutually
exclusive and shared some codepaths.

The prior patch was the trivial bugfix, this is the slightly more involved
cleanup.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Patch 1 was the trivial bugfix, this is the slightly more involved cleanup.

diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index b2113b17e53c..56d9dd787e7b 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -100,7 +100,6 @@ typedef uint16_t	xfs_qwarncnt_t;
 #define XFS_QMOPT_FORCE_RES	0x0000010 /* ignore quota limits */
 #define XFS_QMOPT_SBVERSION	0x0000040 /* change superblock version num */
 #define XFS_QMOPT_GQUOTA	0x0002000 /* group dquot requested */
-#define XFS_QMOPT_ENOSPC	0x0004000 /* enospc instead of edquot (prj) */
 
 /*
  * flags to xfs_trans_mod_dquot to indicate which field needs to be
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index c225691fad15..591779aa2fd0 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1808,7 +1808,7 @@ xfs_qm_vop_chown_reserve(
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	uint64_t		delblks;
-	unsigned int		blkflags, prjflags = 0;
+	unsigned int		blkflags;
 	struct xfs_dquot	*udq_unres = NULL;
 	struct xfs_dquot	*gdq_unres = NULL;
 	struct xfs_dquot	*pdq_unres = NULL;
@@ -1849,7 +1849,6 @@ xfs_qm_vop_chown_reserve(
 
 	if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
 	    ip->i_d.di_projid != be32_to_cpu(pdqp->q_core.d_id)) {
-		prjflags = XFS_QMOPT_ENOSPC;
 		pdq_delblks = pdqp;
 		if (delblks) {
 			ASSERT(ip->i_pdquot);
@@ -1859,8 +1858,7 @@ xfs_qm_vop_chown_reserve(
 
 	error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
 				udq_delblks, gdq_delblks, pdq_delblks,
-				ip->i_d.di_nblocks, 1,
-				flags | blkflags | prjflags);
+				ip->i_d.di_nblocks, 1, flags | blkflags);
 	if (error)
 		return error;
 
@@ -1878,8 +1876,7 @@ xfs_qm_vop_chown_reserve(
 		ASSERT(udq_unres || gdq_unres || pdq_unres);
 		error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
 			    udq_delblks, gdq_delblks, pdq_delblks,
-			    (xfs_qcnt_t)delblks, 0,
-			    flags | blkflags | prjflags);
+			    (xfs_qcnt_t)delblks, 0, flags | blkflags);
 		if (error)
 			return error;
 		xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 2c3557a80e69..2c07897a3c37 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -711,7 +711,7 @@ xfs_trans_dqresv(
 
 error_return:
 	xfs_dqunlock(dqp);
-	if (flags & XFS_QMOPT_ENOSPC)
+	if (XFS_QM_ISPDQ(dqp))
 		return -ENOSPC;
 	return -EDQUOT;
 }
@@ -751,15 +751,13 @@ xfs_trans_reserve_quota_bydquots(
 	ASSERT(flags & XFS_QMOPT_RESBLK_MASK);
 
 	if (udqp) {
-		error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos,
-					(flags & ~XFS_QMOPT_ENOSPC));
+		error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos, flags);
 		if (error)
 			return error;
 	}
 
 	if (gdqp) {
-		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos,
-					(flags & ~XFS_QMOPT_ENOSPC));
+		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags);
 		if (error)
 			goto unwind_usr;
 	}
@@ -804,16 +802,12 @@ xfs_trans_reserve_quota_nblks(
 
 	if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
 		return 0;
-	if (XFS_IS_PQUOTA_ON(mp))
-		flags |= XFS_QMOPT_ENOSPC;
 
 	ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-	ASSERT((flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
-				XFS_TRANS_DQ_RES_RTBLKS ||
-	       (flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
-				XFS_TRANS_DQ_RES_BLKS);
+	ASSERT((flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_RTBLKS ||
+	       (flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_BLKS);
 
 	/*
 	 * Reserve nblks against these dquots, with trans as the mediator.

Re: [PATCH 1/2] xfs: group quota should return EDQUOT when prj quota enabled

[Christoph Hellwig]

On Thu, May 07, 2020 at 10:38:33PM -0500, Eric Sandeen wrote:
> Long ago, group & project quota were mutually exclusive, and so
> when we turned on XFS_QMOPT_ENOSPC ("return ENOSPC if project quota
> is exceeded") when project quota was enabled, we only needed to
> disable it again for user quota.
> 
> When group & project quota got separated, this got missed, and as a
> result if project quota is enabled and group quota is exceeded, the
> error code returned is incorrectly returned as ENOSPC not EDQUOT.
> 
> Fix this by stripping XFS_QMOPT_ENOSPC out of flags for group
> quota when we try to reserve the space.

Looks weird, but given that it has historical precedence, and you fix
all this mess up in the next patch:

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

Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Christoph Hellwig]

On Thu, May 07, 2020 at 11:00:34PM -0500, Eric Sandeen wrote:
> The only place we return -EDQUOT, and therefore need to make a decision
> about returning -ENOSPC for project quota instead, is in xfs_trans_dqresv().
> 
> So there's no reason to be setting and clearing XFS_QMOPT_ENOSPC at higher
> levels; if xfs_trans_dqresv has failed, test if the dqp we were were handed
> is a project quota and if so, return -ENOSPC instead of EDQUOT.  The
> complexity is just a leftover from when project & group quota were mutually
> exclusive and shared some codepaths.
> 
> The prior patch was the trivial bugfix, this is the slightly more involved
> cleanup.

Nice:

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

Re: [PATCH 1/2] xfs: group quota should return EDQUOT when prj quota enabled

[Brian Foster]

On Thu, May 07, 2020 at 10:38:33PM -0500, Eric Sandeen wrote:
> Long ago, group & project quota were mutually exclusive, and so
> when we turned on XFS_QMOPT_ENOSPC ("return ENOSPC if project quota
> is exceeded") when project quota was enabled, we only needed to
> disable it again for user quota.
> 
> When group & project quota got separated, this got missed, and as a
> result if project quota is enabled and group quota is exceeded, the
> error code returned is incorrectly returned as ENOSPC not EDQUOT.
> 
> Fix this by stripping XFS_QMOPT_ENOSPC out of flags for group
> quota when we try to reserve the space.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index e4eca607e512..47c6e88c9db6 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -757,7 +757,8 @@ xfs_trans_reserve_quota_bydquots(
>  	}
>  
>  	if (gdqp) {
> -		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags);
> +		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos,
> +					(flags & ~XFS_QMOPT_ENOSPC));
>  		if (error)
>  			goto unwind_usr;
>  	}
> 

Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Brian Foster]

On Thu, May 07, 2020 at 11:00:34PM -0500, Eric Sandeen wrote:
> The only place we return -EDQUOT, and therefore need to make a decision
> about returning -ENOSPC for project quota instead, is in xfs_trans_dqresv().
> 
> So there's no reason to be setting and clearing XFS_QMOPT_ENOSPC at higher
> levels; if xfs_trans_dqresv has failed, test if the dqp we were were handed
> is a project quota and if so, return -ENOSPC instead of EDQUOT.  The
> complexity is just a leftover from when project & group quota were mutually
> exclusive and shared some codepaths.
> 
> The prior patch was the trivial bugfix, this is the slightly more involved
> cleanup.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Hmm so what about callers that don't pass QMOPT_ENOSPC? For example, it
looks like various xfs_trans_reserve_quota() calls pass a pdqp but don't
set the flag. Is the intent to change behavior such that -ENOSPC is
unconditional for project quota reservation failures?

Brian

> 
> Patch 1 was the trivial bugfix, this is the slightly more involved cleanup.
> 
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index b2113b17e53c..56d9dd787e7b 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -100,7 +100,6 @@ typedef uint16_t	xfs_qwarncnt_t;
>  #define XFS_QMOPT_FORCE_RES	0x0000010 /* ignore quota limits */
>  #define XFS_QMOPT_SBVERSION	0x0000040 /* change superblock version num */
>  #define XFS_QMOPT_GQUOTA	0x0002000 /* group dquot requested */
> -#define XFS_QMOPT_ENOSPC	0x0004000 /* enospc instead of edquot (prj) */
>  
>  /*
>   * flags to xfs_trans_mod_dquot to indicate which field needs to be
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index c225691fad15..591779aa2fd0 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1808,7 +1808,7 @@ xfs_qm_vop_chown_reserve(
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
>  	uint64_t		delblks;
> -	unsigned int		blkflags, prjflags = 0;
> +	unsigned int		blkflags;
>  	struct xfs_dquot	*udq_unres = NULL;
>  	struct xfs_dquot	*gdq_unres = NULL;
>  	struct xfs_dquot	*pdq_unres = NULL;
> @@ -1849,7 +1849,6 @@ xfs_qm_vop_chown_reserve(
>  
>  	if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
>  	    ip->i_d.di_projid != be32_to_cpu(pdqp->q_core.d_id)) {
> -		prjflags = XFS_QMOPT_ENOSPC;
>  		pdq_delblks = pdqp;
>  		if (delblks) {
>  			ASSERT(ip->i_pdquot);
> @@ -1859,8 +1858,7 @@ xfs_qm_vop_chown_reserve(
>  
>  	error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
>  				udq_delblks, gdq_delblks, pdq_delblks,
> -				ip->i_d.di_nblocks, 1,
> -				flags | blkflags | prjflags);
> +				ip->i_d.di_nblocks, 1, flags | blkflags);
>  	if (error)
>  		return error;
>  
> @@ -1878,8 +1876,7 @@ xfs_qm_vop_chown_reserve(
>  		ASSERT(udq_unres || gdq_unres || pdq_unres);
>  		error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
>  			    udq_delblks, gdq_delblks, pdq_delblks,
> -			    (xfs_qcnt_t)delblks, 0,
> -			    flags | blkflags | prjflags);
> +			    (xfs_qcnt_t)delblks, 0, flags | blkflags);
>  		if (error)
>  			return error;
>  		xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 2c3557a80e69..2c07897a3c37 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -711,7 +711,7 @@ xfs_trans_dqresv(
>  
>  error_return:
>  	xfs_dqunlock(dqp);
> -	if (flags & XFS_QMOPT_ENOSPC)
> +	if (XFS_QM_ISPDQ(dqp))
>  		return -ENOSPC;
>  	return -EDQUOT;
>  }
> @@ -751,15 +751,13 @@ xfs_trans_reserve_quota_bydquots(
>  	ASSERT(flags & XFS_QMOPT_RESBLK_MASK);
>  
>  	if (udqp) {
> -		error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos,
> -					(flags & ~XFS_QMOPT_ENOSPC));
> +		error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos, flags);
>  		if (error)
>  			return error;
>  	}
>  
>  	if (gdqp) {
> -		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos,
> -					(flags & ~XFS_QMOPT_ENOSPC));
> +		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags);
>  		if (error)
>  			goto unwind_usr;
>  	}
> @@ -804,16 +802,12 @@ xfs_trans_reserve_quota_nblks(
>  
>  	if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
>  		return 0;
> -	if (XFS_IS_PQUOTA_ON(mp))
> -		flags |= XFS_QMOPT_ENOSPC;
>  
>  	ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
>  
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> -	ASSERT((flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
> -				XFS_TRANS_DQ_RES_RTBLKS ||
> -	       (flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
> -				XFS_TRANS_DQ_RES_BLKS);
> +	ASSERT((flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_RTBLKS ||
> +	       (flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_BLKS);
>  
>  	/*
>  	 * Reserve nblks against these dquots, with trans as the mediator.
> 
> 

Re: [PATCH 1/2] xfs: group quota should return EDQUOT when prj quota enabled

[Eric Sandeen]

On 5/8/20 2:13 AM, Christoph Hellwig wrote:
> On Thu, May 07, 2020 at 10:38:33PM -0500, Eric Sandeen wrote:
>> Long ago, group & project quota were mutually exclusive, and so
>> when we turned on XFS_QMOPT_ENOSPC ("return ENOSPC if project quota
>> is exceeded") when project quota was enabled, we only needed to
>> disable it again for user quota.
>>
>> When group & project quota got separated, this got missed, and as a
>> result if project quota is enabled and group quota is exceeded, the
>> error code returned is incorrectly returned as ENOSPC not EDQUOT.
>>
>> Fix this by stripping XFS_QMOPT_ENOSPC out of flags for group
>> quota when we try to reserve the space.
> 
> Looks weird, but given that it has historical precedence, and you fix
> all this mess up in the next patch:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks.  I could roll it all up into one patch if that's better, this
seemed half weird / half more-obvious....

-Eric

Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Eric Sandeen]

On 5/8/20 8:01 AM, Brian Foster wrote:
> On Thu, May 07, 2020 at 11:00:34PM -0500, Eric Sandeen wrote:
>> The only place we return -EDQUOT, and therefore need to make a decision
>> about returning -ENOSPC for project quota instead, is in xfs_trans_dqresv().
>>
>> So there's no reason to be setting and clearing XFS_QMOPT_ENOSPC at higher
>> levels; if xfs_trans_dqresv has failed, test if the dqp we were were handed
>> is a project quota and if so, return -ENOSPC instead of EDQUOT.  The
>> complexity is just a leftover from when project & group quota were mutually
>> exclusive and shared some codepaths.
>>
>> The prior patch was the trivial bugfix, this is the slightly more involved
>> cleanup.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
> 
> Hmm so what about callers that don't pass QMOPT_ENOSPC? For example, it
> looks like various xfs_trans_reserve_quota() calls pass a pdqp but don't
> set the flag.

Oh, interesting.  I bet that was an oversight, tbh, but let's see.

<rewinds 14 years>

commit 9a2a7de268f67fea0c450ed3e99a2d31f43d7166
Author: Nathan Scott <nathans@sgi.com>
Date:   Fri Mar 31 13:04:49 2006 +1000

    [XFS] Make project quota enforcement return an error code consistent with
    its use.

so yeah, even back then, stuff like xfs_symlink returned EDQUOT not ENOSPC.

Today, these call the reservation w/o the special ENOSPC flag:

xfs_unmap_extent
xfs_create
xfs_create_tmpfile
xfs_symlink

and so will return EDQUOT instead of ENOSPC even for project quota.

You're right that my patch changes these to ENOSPC.

> Is the intent to change behavior such that -ENOSPC is
> unconditional for project quota reservation failures?

Now it's a conundrum.  I /think/ the current behavior is due to an oversight, but 

a) I'm not certain, and
b) can we change it now?

-Eric

Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Brian Foster]

On Fri, May 08, 2020 at 10:45:48AM -0500, Eric Sandeen wrote:
> On 5/8/20 8:01 AM, Brian Foster wrote:
> > On Thu, May 07, 2020 at 11:00:34PM -0500, Eric Sandeen wrote:
> >> The only place we return -EDQUOT, and therefore need to make a decision
> >> about returning -ENOSPC for project quota instead, is in xfs_trans_dqresv().
> >>
> >> So there's no reason to be setting and clearing XFS_QMOPT_ENOSPC at higher
> >> levels; if xfs_trans_dqresv has failed, test if the dqp we were were handed
> >> is a project quota and if so, return -ENOSPC instead of EDQUOT.  The
> >> complexity is just a leftover from when project & group quota were mutually
> >> exclusive and shared some codepaths.
> >>
> >> The prior patch was the trivial bugfix, this is the slightly more involved
> >> cleanup.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> > 
> > Hmm so what about callers that don't pass QMOPT_ENOSPC? For example, it
> > looks like various xfs_trans_reserve_quota() calls pass a pdqp but don't
> > set the flag.
> 
> Oh, interesting.  I bet that was an oversight, tbh, but let's see.
> 
> <rewinds 14 years>
> 
> commit 9a2a7de268f67fea0c450ed3e99a2d31f43d7166
> Author: Nathan Scott <nathans@sgi.com>
> Date:   Fri Mar 31 13:04:49 2006 +1000
> 
>     [XFS] Make project quota enforcement return an error code consistent with
>     its use.
> 
> so yeah, even back then, stuff like xfs_symlink returned EDQUOT not ENOSPC.
> 
> Today, these call the reservation w/o the special ENOSPC flag:
> 
> xfs_unmap_extent
> xfs_create
> xfs_create_tmpfile
> xfs_symlink
> 
> and so will return EDQUOT instead of ENOSPC even for project quota.
> 
> You're right that my patch changes these to ENOSPC.
> 
> > Is the intent to change behavior such that -ENOSPC is
> > unconditional for project quota reservation failures?
> 
> Now it's a conundrum.  I /think/ the current behavior is due to an oversight, but 
> 
> a) I'm not certain, and
> b) can we change it now?
> 

Heh, I can't really tell what the intended/expected behavior is. For
whatever it's worth, it seems reasonable enough to me to change it based
on the fact that project quotas have been expected to return -ENOSPC in
at least some common cases for many years. It seems unlikely that users
would know or care about the change in behavior in the subset noted
above, but who knows. It might be good to get some other opinions. :P

Brian

> -Eric
> 

Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Darrick J. Wong]

On Fri, May 08, 2020 at 12:21:29PM -0400, Brian Foster wrote:
> On Fri, May 08, 2020 at 10:45:48AM -0500, Eric Sandeen wrote:
> > On 5/8/20 8:01 AM, Brian Foster wrote:
> > > On Thu, May 07, 2020 at 11:00:34PM -0500, Eric Sandeen wrote:
> > >> The only place we return -EDQUOT, and therefore need to make a decision
> > >> about returning -ENOSPC for project quota instead, is in xfs_trans_dqresv().
> > >>
> > >> So there's no reason to be setting and clearing XFS_QMOPT_ENOSPC at higher
> > >> levels; if xfs_trans_dqresv has failed, test if the dqp we were were handed
> > >> is a project quota and if so, return -ENOSPC instead of EDQUOT.  The
> > >> complexity is just a leftover from when project & group quota were mutually
> > >> exclusive and shared some codepaths.
> > >>
> > >> The prior patch was the trivial bugfix, this is the slightly more involved
> > >> cleanup.
> > >>
> > >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > >> ---
> > > 
> > > Hmm so what about callers that don't pass QMOPT_ENOSPC? For example, it
> > > looks like various xfs_trans_reserve_quota() calls pass a pdqp but don't
> > > set the flag.
> > 
> > Oh, interesting.  I bet that was an oversight, tbh, but let's see.
> > 
> > <rewinds 14 years>
> > 
> > commit 9a2a7de268f67fea0c450ed3e99a2d31f43d7166
> > Author: Nathan Scott <nathans@sgi.com>
> > Date:   Fri Mar 31 13:04:49 2006 +1000
> > 
> >     [XFS] Make project quota enforcement return an error code consistent with
> >     its use.
> > 
> > so yeah, even back then, stuff like xfs_symlink returned EDQUOT not ENOSPC.
> > 
> > Today, these call the reservation w/o the special ENOSPC flag:
> > 
> > xfs_unmap_extent
> > xfs_create
> > xfs_create_tmpfile
> > xfs_symlink
> > 
> > and so will return EDQUOT instead of ENOSPC even for project quota.
> > 
> > You're right that my patch changes these to ENOSPC.
> > 
> > > Is the intent to change behavior such that -ENOSPC is
> > > unconditional for project quota reservation failures?
> > 
> > Now it's a conundrum.  I /think/ the current behavior is due to an oversight, but 
> > 
> > a) I'm not certain, and
> > b) can we change it now?
> > 
> 
> Heh, I can't really tell what the intended/expected behavior is. For
> whatever it's worth, it seems reasonable enough to me to change it based
> on the fact that project quotas have been expected to return -ENOSPC in
> at least some common cases for many years. It seems unlikely that users
> would know or care about the change in behavior in the subset noted
> above, but who knows. It might be good to get some other opinions. :P

"I bet you a beer at the next conference (if they ever happen again)
that nobody will notice"? :P

TBH while I find it a little odd that project quota gets to return
ENOSPC instead of EDQUOT, I find it more odd that sometimes it doesn't.
This at least gets us to consistent behavior (EDQUOT for user/group,
ENOSPC for project) so for the series:

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

(Let's see what an fstests run spits out...)

--D

> Brian
> 
> > -Eric
> > 
> 

Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Brian Foster]

On Tue, May 12, 2020 at 04:34:43PM -0700, Darrick J. Wong wrote:
> On Fri, May 08, 2020 at 12:21:29PM -0400, Brian Foster wrote:
> > On Fri, May 08, 2020 at 10:45:48AM -0500, Eric Sandeen wrote:
> > > On 5/8/20 8:01 AM, Brian Foster wrote:
> > > > On Thu, May 07, 2020 at 11:00:34PM -0500, Eric Sandeen wrote:
> > > >> The only place we return -EDQUOT, and therefore need to make a decision
> > > >> about returning -ENOSPC for project quota instead, is in xfs_trans_dqresv().
> > > >>
> > > >> So there's no reason to be setting and clearing XFS_QMOPT_ENOSPC at higher
> > > >> levels; if xfs_trans_dqresv has failed, test if the dqp we were were handed
> > > >> is a project quota and if so, return -ENOSPC instead of EDQUOT.  The
> > > >> complexity is just a leftover from when project & group quota were mutually
> > > >> exclusive and shared some codepaths.
> > > >>
> > > >> The prior patch was the trivial bugfix, this is the slightly more involved
> > > >> cleanup.
> > > >>
> > > >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > > >> ---
> > > > 
> > > > Hmm so what about callers that don't pass QMOPT_ENOSPC? For example, it
> > > > looks like various xfs_trans_reserve_quota() calls pass a pdqp but don't
> > > > set the flag.
> > > 
> > > Oh, interesting.  I bet that was an oversight, tbh, but let's see.
> > > 
> > > <rewinds 14 years>
> > > 
> > > commit 9a2a7de268f67fea0c450ed3e99a2d31f43d7166
> > > Author: Nathan Scott <nathans@sgi.com>
> > > Date:   Fri Mar 31 13:04:49 2006 +1000
> > > 
> > >     [XFS] Make project quota enforcement return an error code consistent with
> > >     its use.
> > > 
> > > so yeah, even back then, stuff like xfs_symlink returned EDQUOT not ENOSPC.
> > > 
> > > Today, these call the reservation w/o the special ENOSPC flag:
> > > 
> > > xfs_unmap_extent
> > > xfs_create
> > > xfs_create_tmpfile
> > > xfs_symlink
> > > 
> > > and so will return EDQUOT instead of ENOSPC even for project quota.
> > > 
> > > You're right that my patch changes these to ENOSPC.
> > > 
> > > > Is the intent to change behavior such that -ENOSPC is
> > > > unconditional for project quota reservation failures?
> > > 
> > > Now it's a conundrum.  I /think/ the current behavior is due to an oversight, but 
> > > 
> > > a) I'm not certain, and
> > > b) can we change it now?
> > > 
> > 
> > Heh, I can't really tell what the intended/expected behavior is. For
> > whatever it's worth, it seems reasonable enough to me to change it based
> > on the fact that project quotas have been expected to return -ENOSPC in
> > at least some common cases for many years. It seems unlikely that users
> > would know or care about the change in behavior in the subset noted
> > above, but who knows. It might be good to get some other opinions. :P
> 
> "I bet you a beer at the next conference (if they ever happen again)
> that nobody will notice"? :P
> 

Apocalypse aside, free beer is free beer. ;)

> TBH while I find it a little odd that project quota gets to return
> ENOSPC instead of EDQUOT, I find it more odd that sometimes it doesn't.
> This at least gets us to consistent behavior (EDQUOT for user/group,
> ENOSPC for project) so for the series:
> 

Works for me, but can we update the commit log to describe the behavior
change before this goes in? In fact, it might even make sense to retitle
the patch to something like "xfs: always return -ENOSPC on project quota
reservation failure" and let the flag removal be a side effect of that.

Brian

> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> (Let's see what an fstests run spits out...)
> 
> --D
> 
> > Brian
> > 
> > > -Eric
> > > 
> > 
> 

Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Eric Sandeen]

On 5/13/20 6:00 AM, Brian Foster wrote:
> On Tue, May 12, 2020 at 04:34:43PM -0700, Darrick J. Wong wrote:
>> On Fri, May 08, 2020 at 12:21:29PM -0400, Brian Foster wrote:
>>> On Fri, May 08, 2020 at 10:45:48AM -0500, Eric Sandeen wrote:
>>>> On 5/8/20 8:01 AM, Brian Foster wrote:

...

>>>> You're right that my patch changes these to ENOSPC.
>>>>
>>>>> Is the intent to change behavior such that -ENOSPC is
>>>>> unconditional for project quota reservation failures?
>>>>
>>>> Now it's a conundrum.  I /think/ the current behavior is due to an oversight, but 
>>>>
>>>> a) I'm not certain, and
>>>> b) can we change it now?
>>>>
>>>
>>> Heh, I can't really tell what the intended/expected behavior is. For
>>> whatever it's worth, it seems reasonable enough to me to change it based
>>> on the fact that project quotas have been expected to return -ENOSPC in
>>> at least some common cases for many years. It seems unlikely that users
>>> would know or care about the change in behavior in the subset noted
>>> above, but who knows. It might be good to get some other opinions. :P
>>
>> "I bet you a beer at the next conference (if they ever happen again)
>> that nobody will notice"? :P
>>
> 
> Apocalypse aside, free beer is free beer. ;)
> 
>> TBH while I find it a little odd that project quota gets to return
>> ENOSPC instead of EDQUOT, I find it more odd that sometimes it doesn't.
>> This at least gets us to consistent behavior (EDQUOT for user/group,
>> ENOSPC for project) so for the series:
>>
> 
> Works for me, but can we update the commit log to describe the behavior
> change before this goes in? In fact, it might even make sense to retitle
> the patch to something like "xfs: always return -ENOSPC on project quota
> reservation failure" and let the flag removal be a side effect of that.

Yes that's a good plan. I'm also happy to just combine the 2 patches if
that's better.  I'll sync up w/ Darrick to see if this can still happen.e

Thanks again for spotting the difference,

-Eric

> Brian

Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

[Darrick J. Wong]

On Wed, May 13, 2020 at 08:39:47AM -0500, Eric Sandeen wrote:
> On 5/13/20 6:00 AM, Brian Foster wrote:
> > On Tue, May 12, 2020 at 04:34:43PM -0700, Darrick J. Wong wrote:
> >> On Fri, May 08, 2020 at 12:21:29PM -0400, Brian Foster wrote:
> >>> On Fri, May 08, 2020 at 10:45:48AM -0500, Eric Sandeen wrote:
> >>>> On 5/8/20 8:01 AM, Brian Foster wrote:
> 
> ...
> 
> >>>> You're right that my patch changes these to ENOSPC.
> >>>>
> >>>>> Is the intent to change behavior such that -ENOSPC is
> >>>>> unconditional for project quota reservation failures?
> >>>>
> >>>> Now it's a conundrum.  I /think/ the current behavior is due to an oversight, but 
> >>>>
> >>>> a) I'm not certain, and
> >>>> b) can we change it now?
> >>>>
> >>>
> >>> Heh, I can't really tell what the intended/expected behavior is. For
> >>> whatever it's worth, it seems reasonable enough to me to change it based
> >>> on the fact that project quotas have been expected to return -ENOSPC in
> >>> at least some common cases for many years. It seems unlikely that users
> >>> would know or care about the change in behavior in the subset noted
> >>> above, but who knows. It might be good to get some other opinions. :P
> >>
> >> "I bet you a beer at the next conference (if they ever happen again)
> >> that nobody will notice"? :P
> >>
> > 
> > Apocalypse aside, free beer is free beer. ;)
> > 
> >> TBH while I find it a little odd that project quota gets to return
> >> ENOSPC instead of EDQUOT, I find it more odd that sometimes it doesn't.
> >> This at least gets us to consistent behavior (EDQUOT for user/group,
> >> ENOSPC for project) so for the series:
> >>
> > 
> > Works for me, but can we update the commit log to describe the behavior
> > change before this goes in? In fact, it might even make sense to retitle
> > the patch to something like "xfs: always return -ENOSPC on project quota
> > reservation failure" and let the flag removal be a side effect of that.
> 
> Yes that's a good plan. I'm also happy to just combine the 2 patches if
> that's better.  I'll sync up w/ Darrick to see if this can still happen.e
> 
> Thanks again for spotting the difference,

Yeah, that's fine.  I haven't even had a chance to find out if last
night's testing passed... :$

--D

> -Eric
> 
> > Brian

[PATCH 2/2 V2] xfs: always return -ENOSPC on project quota reservation failure

[Eric Sandeen]

XFS project quota treats project hierarchies as "mini filesysems" and
so rather than -EDQUOT, the intent is to return -ENOSPC when a quota
reservation fails, but this behavior is not consistent.

The only place we make a decision between -EDQUOT and -ENOSPC
returns based on quota type is in xfs_trans_dqresv().

This behavior is currently controlled by whether or not the
XFS_QMOPT_ENOSPC flag gets passed into the quota reservation.  However,
its use is not consistent; paths such as xfs_create() and xfs_symlink()
don't set the flag, so a reservation failure will return -EDQUOT for
project quota reservation failures rather than -ENOSPC for these sorts
of operations, even for project quota:

# mkdir mnt/project
# xfs_quota -x -c "project -s -p mnt/project 42" mnt
# xfs_quota -x -c 'limit -p isoft=2 ihard=3 42' mnt
# touch mnt/project/file{1,2,3}
touch: cannot touch ‘mnt/project/file3’: Disk quota exceeded

We can make this consistent by not requiring the flag to be set at the
top of the callchain; instead we can simply test whether we are
reserving a project quota with XFS_QM_ISPDQ in xfs_trans_dqresv and if
so, return -ENOSPC for that failure.  This removes the need for the
XFS_QMOPT_ENOSPC altogether and simplifies the code a fair bit.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

V2: Change title and changelog to clearly reflect the bug and behavior
change, which wasn't really identified as a bug and behavior change until
Brian carefully reviewed V1.  :)

No patch changes.

diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index b2113b17e53c..56d9dd787e7b 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -100,7 +100,6 @@ typedef uint16_t	xfs_qwarncnt_t;
 #define XFS_QMOPT_FORCE_RES	0x0000010 /* ignore quota limits */
 #define XFS_QMOPT_SBVERSION	0x0000040 /* change superblock version num */
 #define XFS_QMOPT_GQUOTA	0x0002000 /* group dquot requested */
-#define XFS_QMOPT_ENOSPC	0x0004000 /* enospc instead of edquot (prj) */
 
 /*
  * flags to xfs_trans_mod_dquot to indicate which field needs to be
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index c225691fad15..591779aa2fd0 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1808,7 +1808,7 @@ xfs_qm_vop_chown_reserve(
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	uint64_t		delblks;
-	unsigned int		blkflags, prjflags = 0;
+	unsigned int		blkflags;
 	struct xfs_dquot	*udq_unres = NULL;
 	struct xfs_dquot	*gdq_unres = NULL;
 	struct xfs_dquot	*pdq_unres = NULL;
@@ -1849,7 +1849,6 @@ xfs_qm_vop_chown_reserve(
 
 	if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
 	    ip->i_d.di_projid != be32_to_cpu(pdqp->q_core.d_id)) {
-		prjflags = XFS_QMOPT_ENOSPC;
 		pdq_delblks = pdqp;
 		if (delblks) {
 			ASSERT(ip->i_pdquot);
@@ -1859,8 +1858,7 @@ xfs_qm_vop_chown_reserve(
 
 	error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
 				udq_delblks, gdq_delblks, pdq_delblks,
-				ip->i_d.di_nblocks, 1,
-				flags | blkflags | prjflags);
+				ip->i_d.di_nblocks, 1, flags | blkflags);
 	if (error)
 		return error;
 
@@ -1878,8 +1876,7 @@ xfs_qm_vop_chown_reserve(
 		ASSERT(udq_unres || gdq_unres || pdq_unres);
 		error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
 			    udq_delblks, gdq_delblks, pdq_delblks,
-			    (xfs_qcnt_t)delblks, 0,
-			    flags | blkflags | prjflags);
+			    (xfs_qcnt_t)delblks, 0, flags | blkflags);
 		if (error)
 			return error;
 		xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 2c3557a80e69..2c07897a3c37 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -711,7 +711,7 @@ xfs_trans_dqresv(
 
 error_return:
 	xfs_dqunlock(dqp);
-	if (flags & XFS_QMOPT_ENOSPC)
+	if (XFS_QM_ISPDQ(dqp))
 		return -ENOSPC;
 	return -EDQUOT;
 }
@@ -751,15 +751,13 @@ xfs_trans_reserve_quota_bydquots(
 	ASSERT(flags & XFS_QMOPT_RESBLK_MASK);
 
 	if (udqp) {
-		error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos,
-					(flags & ~XFS_QMOPT_ENOSPC));
+		error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos, flags);
 		if (error)
 			return error;
 	}
 
 	if (gdqp) {
-		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos,
-					(flags & ~XFS_QMOPT_ENOSPC));
+		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags);
 		if (error)
 			goto unwind_usr;
 	}
@@ -804,16 +802,12 @@ xfs_trans_reserve_quota_nblks(
 
 	if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
 		return 0;
-	if (XFS_IS_PQUOTA_ON(mp))
-		flags |= XFS_QMOPT_ENOSPC;
 
 	ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-	ASSERT((flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
-				XFS_TRANS_DQ_RES_RTBLKS ||
-	       (flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
-				XFS_TRANS_DQ_RES_BLKS);
+	ASSERT((flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_RTBLKS ||
+	       (flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_BLKS);
 
 	/*
 	 * Reserve nblks against these dquots, with trans as the mediator.

Re: [PATCH 2/2 V2] xfs: always return -ENOSPC on project quota reservation failure

[Brian Foster]

On Wed, May 13, 2020 at 12:39:00PM -0500, Eric Sandeen wrote:
> XFS project quota treats project hierarchies as "mini filesysems" and
> so rather than -EDQUOT, the intent is to return -ENOSPC when a quota
> reservation fails, but this behavior is not consistent.
> 
> The only place we make a decision between -EDQUOT and -ENOSPC
> returns based on quota type is in xfs_trans_dqresv().
> 
> This behavior is currently controlled by whether or not the
> XFS_QMOPT_ENOSPC flag gets passed into the quota reservation.  However,
> its use is not consistent; paths such as xfs_create() and xfs_symlink()
> don't set the flag, so a reservation failure will return -EDQUOT for
> project quota reservation failures rather than -ENOSPC for these sorts
> of operations, even for project quota:
> 
> # mkdir mnt/project
> # xfs_quota -x -c "project -s -p mnt/project 42" mnt
> # xfs_quota -x -c 'limit -p isoft=2 ihard=3 42' mnt
> # touch mnt/project/file{1,2,3}
> touch: cannot touch ‘mnt/project/file3’: Disk quota exceeded
> 
> We can make this consistent by not requiring the flag to be set at the
> top of the callchain; instead we can simply test whether we are
> reserving a project quota with XFS_QM_ISPDQ in xfs_trans_dqresv and if
> so, return -ENOSPC for that failure.  This removes the need for the
> XFS_QMOPT_ENOSPC altogether and simplifies the code a fair bit.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---

Thanks for the update.

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
> V2: Change title and changelog to clearly reflect the bug and behavior
> change, which wasn't really identified as a bug and behavior change until
> Brian carefully reviewed V1.  :)
> 
> No patch changes.
> 
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index b2113b17e53c..56d9dd787e7b 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -100,7 +100,6 @@ typedef uint16_t	xfs_qwarncnt_t;
>  #define XFS_QMOPT_FORCE_RES	0x0000010 /* ignore quota limits */
>  #define XFS_QMOPT_SBVERSION	0x0000040 /* change superblock version num */
>  #define XFS_QMOPT_GQUOTA	0x0002000 /* group dquot requested */
> -#define XFS_QMOPT_ENOSPC	0x0004000 /* enospc instead of edquot (prj) */
>  
>  /*
>   * flags to xfs_trans_mod_dquot to indicate which field needs to be
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index c225691fad15..591779aa2fd0 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1808,7 +1808,7 @@ xfs_qm_vop_chown_reserve(
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
>  	uint64_t		delblks;
> -	unsigned int		blkflags, prjflags = 0;
> +	unsigned int		blkflags;
>  	struct xfs_dquot	*udq_unres = NULL;
>  	struct xfs_dquot	*gdq_unres = NULL;
>  	struct xfs_dquot	*pdq_unres = NULL;
> @@ -1849,7 +1849,6 @@ xfs_qm_vop_chown_reserve(
>  
>  	if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
>  	    ip->i_d.di_projid != be32_to_cpu(pdqp->q_core.d_id)) {
> -		prjflags = XFS_QMOPT_ENOSPC;
>  		pdq_delblks = pdqp;
>  		if (delblks) {
>  			ASSERT(ip->i_pdquot);
> @@ -1859,8 +1858,7 @@ xfs_qm_vop_chown_reserve(
>  
>  	error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
>  				udq_delblks, gdq_delblks, pdq_delblks,
> -				ip->i_d.di_nblocks, 1,
> -				flags | blkflags | prjflags);
> +				ip->i_d.di_nblocks, 1, flags | blkflags);
>  	if (error)
>  		return error;
>  
> @@ -1878,8 +1876,7 @@ xfs_qm_vop_chown_reserve(
>  		ASSERT(udq_unres || gdq_unres || pdq_unres);
>  		error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
>  			    udq_delblks, gdq_delblks, pdq_delblks,
> -			    (xfs_qcnt_t)delblks, 0,
> -			    flags | blkflags | prjflags);
> +			    (xfs_qcnt_t)delblks, 0, flags | blkflags);
>  		if (error)
>  			return error;
>  		xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index 2c3557a80e69..2c07897a3c37 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -711,7 +711,7 @@ xfs_trans_dqresv(
>  
>  error_return:
>  	xfs_dqunlock(dqp);
> -	if (flags & XFS_QMOPT_ENOSPC)
> +	if (XFS_QM_ISPDQ(dqp))
>  		return -ENOSPC;
>  	return -EDQUOT;
>  }
> @@ -751,15 +751,13 @@ xfs_trans_reserve_quota_bydquots(
>  	ASSERT(flags & XFS_QMOPT_RESBLK_MASK);
>  
>  	if (udqp) {
> -		error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos,
> -					(flags & ~XFS_QMOPT_ENOSPC));
> +		error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos, flags);
>  		if (error)
>  			return error;
>  	}
>  
>  	if (gdqp) {
> -		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos,
> -					(flags & ~XFS_QMOPT_ENOSPC));
> +		error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags);
>  		if (error)
>  			goto unwind_usr;
>  	}
> @@ -804,16 +802,12 @@ xfs_trans_reserve_quota_nblks(
>  
>  	if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
>  		return 0;
> -	if (XFS_IS_PQUOTA_ON(mp))
> -		flags |= XFS_QMOPT_ENOSPC;
>  
>  	ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
>  
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> -	ASSERT((flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
> -				XFS_TRANS_DQ_RES_RTBLKS ||
> -	       (flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
> -				XFS_TRANS_DQ_RES_BLKS);
> +	ASSERT((flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_RTBLKS ||
> +	       (flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_BLKS);
>  
>  	/*
>  	 * Reserve nblks against these dquots, with trans as the mediator.
> 
> 
>