[PATCH] xfs: retry dqpurge when dquot buffer is busy

[PATCH] xfs: retry dqpurge when dquot buffer is busy

[Yingjie Gao]

xfs_qm_dqpurge() marks a zero-reference dquot dead before trying to flush
a dirty dquot. If the attached buffer is busy, xfs_dquot_use_attached_buf()
returns -EAGAIN.

The error path restores q_lockref.count but then jumps to out_funlock,
which continues into the successful purge tail and destroys the dquot.  At
that point the attached buffer has not been detached and the dquot log item
may still be in the AIL.

Restore the retry behavior by dropping the locks and returning -EAGAIN
after resurrecting the lockref.

Link: https://lore.kernel.org/linux-xfs/20260625175519.GF6078@frogsfrogsfrogs/
Fixes: 0c5e80bd579f ("xfs: use a lockref for the xfs_dquot reference count")
Cc: stable@vger.kernel.org # v6.19+
Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
---
 fs/xfs/xfs_qm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index aa0d2976f1c3..0622c72292d8 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -155,8 +155,12 @@ xfs_qm_dqpurge(
 		error = xfs_dquot_use_attached_buf(dqp, &bp);
 		if (error == -EAGAIN) {
 			/* resurrect the refcount from the dead. */
+			xfs_dqfunlock(dqp);
+			mutex_unlock(&dqp->q_qlock);
+			spin_lock(&dqp->q_lockref.lock);
 			dqp->q_lockref.count = 0;
-			goto out_funlock;
+			spin_unlock(&dqp->q_lockref.lock);
+			return -EAGAIN;
 		}
 		if (!bp)
 			goto out_funlock;
-- 
2.20.1

Re: [PATCH] xfs: retry dqpurge when dquot buffer is busy

[Darrick J. Wong]

On Fri, Jun 26, 2026 at 05:52:53PM +0800, Yingjie Gao wrote:
> xfs_qm_dqpurge() marks a zero-reference dquot dead before trying to flush
> a dirty dquot. If the attached buffer is busy, xfs_dquot_use_attached_buf()
> returns -EAGAIN.
> 
> The error path restores q_lockref.count but then jumps to out_funlock,
> which continues into the successful purge tail and destroys the dquot.  At
> that point the attached buffer has not been detached and the dquot log item
> may still be in the AIL.
> 
> Restore the retry behavior by dropping the locks and returning -EAGAIN
> after resurrecting the lockref.
> 
> Link: https://lore.kernel.org/linux-xfs/20260625175519.GF6078@frogsfrogsfrogs/
> Fixes: 0c5e80bd579f ("xfs: use a lockref for the xfs_dquot reference count")
> Cc: stable@vger.kernel.org # v6.19+
> Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>

Yeah, that's more like what we did before 0c5e80bd579f.  I think the
lockref resurrection part still looks ok, but maybe hch has an opinion?

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_qm.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index aa0d2976f1c3..0622c72292d8 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -155,8 +155,12 @@ xfs_qm_dqpurge(
>  		error = xfs_dquot_use_attached_buf(dqp, &bp);
>  		if (error == -EAGAIN) {
>  			/* resurrect the refcount from the dead. */
> +			xfs_dqfunlock(dqp);
> +			mutex_unlock(&dqp->q_qlock);
> +			spin_lock(&dqp->q_lockref.lock);
>  			dqp->q_lockref.count = 0;
> -			goto out_funlock;
> +			spin_unlock(&dqp->q_lockref.lock);
> +			return -EAGAIN;
>  		}
>  		if (!bp)
>  			goto out_funlock;
> -- 
> 2.20.1
> 
> 

Re: [PATCH] xfs: retry dqpurge when dquot buffer is busy

[Christoph Hellwig]

On Fri, Jun 26, 2026 at 01:52:57PM -0700, Darrick J. Wong wrote:
> On Fri, Jun 26, 2026 at 05:52:53PM +0800, Yingjie Gao wrote:
> > xfs_qm_dqpurge() marks a zero-reference dquot dead before trying to flush
> > a dirty dquot. If the attached buffer is busy, xfs_dquot_use_attached_buf()
> > returns -EAGAIN.
> > 
> > The error path restores q_lockref.count but then jumps to out_funlock,
> > which continues into the successful purge tail and destroys the dquot.  At
> > that point the attached buffer has not been detached and the dquot log item
> > may still be in the AIL.
> > 
> > Restore the retry behavior by dropping the locks and returning -EAGAIN
> > after resurrecting the lockref.
> > 
> > Link: https://lore.kernel.org/linux-xfs/20260625175519.GF6078@frogsfrogsfrogs/
> > Fixes: 0c5e80bd579f ("xfs: use a lockref for the xfs_dquot reference count")
> > Cc: stable@vger.kernel.org # v6.19+
> > Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
> 
> Yeah, that's more like what we did before 0c5e80bd579f.  I think the
> lockref resurrection part still looks ok, but maybe hch has an opinion?

But is it the right thing?  In dqpurge we really want to kill of the
dquot, so doing a trylock is not very useful, so we really should not
do a trylock here but just lock the buffer.

Darrick, do you remember if there's any lock order inversions we need
to care about here?

Re: [PATCH] xfs: retry dqpurge when dquot buffer is busy

[Darrick J. Wong]

On Mon, Jun 29, 2026 at 02:43:56PM +0200, Christoph Hellwig wrote:
> On Fri, Jun 26, 2026 at 01:52:57PM -0700, Darrick J. Wong wrote:
> > On Fri, Jun 26, 2026 at 05:52:53PM +0800, Yingjie Gao wrote:
> > > xfs_qm_dqpurge() marks a zero-reference dquot dead before trying to flush
> > > a dirty dquot. If the attached buffer is busy, xfs_dquot_use_attached_buf()
> > > returns -EAGAIN.
> > > 
> > > The error path restores q_lockref.count but then jumps to out_funlock,
> > > which continues into the successful purge tail and destroys the dquot.  At
> > > that point the attached buffer has not been detached and the dquot log item
> > > may still be in the AIL.
> > > 
> > > Restore the retry behavior by dropping the locks and returning -EAGAIN
> > > after resurrecting the lockref.
> > > 
> > > Link: https://lore.kernel.org/linux-xfs/20260625175519.GF6078@frogsfrogsfrogs/
> > > Fixes: 0c5e80bd579f ("xfs: use a lockref for the xfs_dquot reference count")
> > > Cc: stable@vger.kernel.org # v6.19+
> > > Signed-off-by: Yingjie Gao <gaoyingjie@uniontech.com>
> > 
> > Yeah, that's more like what we did before 0c5e80bd579f.  I think the
> > lockref resurrection part still looks ok, but maybe hch has an opinion?
> 
> But is it the right thing?  In dqpurge we really want to kill of the
> dquot, so doing a trylock is not very useful, so we really should not
> do a trylock here but just lock the buffer.
> 
> Darrick, do you remember if there's any lock order inversions we need
> to care about here?

I don't remember off the top of my head, other than (afaict) the trylock
might be a remnant of not wanting to block in quotaoff purge while the
QUOTAOFF log item is also pinning the log tail.

--D