| Message ID | 159353171640.2864738.7967439700762859129.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: remove xfs_disk_quot from incore dquot | expand |
On 6/30/20 8:41 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > In commit 8d3d7e2b35ea, we changed xfs_qm_dqpurge to bail out if we > can't lock the dquot buf to flush the dquot. This prevents the AIL from > blocking on the dquot, but it also forgets to clear the FREEING flag on > its way out. A subsequent purge attempt will see the FREEING flag is > set and bail out, which leads to dqpurge_all failing to purge all the > dquots. This causes unmounts and quotaoff operations to hang. > > Fixes: 8d3d7e2b35ea ("xfs: trylock underlying buffer on dquot flush") > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Ok Reviewed-by: Allison Collins <allison.henderson@oracle.com> > --- > fs/xfs/xfs_qm.c | 1 + > 1 file changed, 1 insertion(+) > > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > index d6cd83317344..938023dd8ce5 100644 > --- a/fs/xfs/xfs_qm.c > +++ b/fs/xfs/xfs_qm.c > @@ -148,6 +148,7 @@ xfs_qm_dqpurge( > error = xfs_bwrite(bp); > xfs_buf_relse(bp); > } else if (error == -EAGAIN) { > + dqp->dq_flags &= ~XFS_DQ_FREEING; > goto out_unlock; > } > xfs_dqflock(dqp); >
On Tuesday 30 June 2020 9:11:56 PM IST Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > In commit 8d3d7e2b35ea, we changed xfs_qm_dqpurge to bail out if we > can't lock the dquot buf to flush the dquot. This prevents the AIL from > blocking on the dquot, but it also forgets to clear the FREEING flag on > its way out. A subsequent purge attempt will see the FREEING flag is > set and bail out, which leads to dqpurge_all failing to purge all the > dquots. This causes unmounts and quotaoff operations to hang. > xfs_qm_dqpurge() checks for the presence of XFS_DQ_FREEING. If it is set, it indicates that another task is freeing this dquot. A failed read operation could return -EAGAIN and hence xfs_qm_dqpurge() returning without clearing XFS_DQ_FREEING would mean that future invocations of this function would turn out to be a nop and hence the corresponding dquot would linger around forever. Hence the fix is correct. Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > Fixes: 8d3d7e2b35ea ("xfs: trylock underlying buffer on dquot flush") > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/xfs_qm.c | 1 + > 1 file changed, 1 insertion(+) > > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > index d6cd83317344..938023dd8ce5 100644 > --- a/fs/xfs/xfs_qm.c > +++ b/fs/xfs/xfs_qm.c > @@ -148,6 +148,7 @@ xfs_qm_dqpurge( > error = xfs_bwrite(bp); > xfs_buf_relse(bp); > } else if (error == -EAGAIN) { > + dqp->dq_flags &= ~XFS_DQ_FREEING; > goto out_unlock; > } > xfs_dqflock(dqp); > >
On Tue, Jun 30, 2020 at 08:41:56AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > In commit 8d3d7e2b35ea, we changed xfs_qm_dqpurge to bail out if we > can't lock the dquot buf to flush the dquot. This prevents the AIL from > blocking on the dquot, but it also forgets to clear the FREEING flag on > its way out. A subsequent purge attempt will see the FREEING flag is > set and bail out, which leads to dqpurge_all failing to purge all the > dquots. This causes unmounts and quotaoff operations to hang. > > Fixes: 8d3d7e2b35ea ("xfs: trylock underlying buffer on dquot flush") > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Looks like Dave submitted this as a separate fix just after you.. Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Jul 01, 2020 at 09:33:58AM +0100, Christoph Hellwig wrote: > On Tue, Jun 30, 2020 at 08:41:56AM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > In commit 8d3d7e2b35ea, we changed xfs_qm_dqpurge to bail out if we > > can't lock the dquot buf to flush the dquot. This prevents the AIL from > > blocking on the dquot, but it also forgets to clear the FREEING flag on > > its way out. A subsequent purge attempt will see the FREEING flag is > > set and bail out, which leads to dqpurge_all failing to purge all the > > dquots. This causes unmounts and quotaoff operations to hang. > > > > Fixes: 8d3d7e2b35ea ("xfs: trylock underlying buffer on dquot flush") > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > Looks like Dave submitted this as a separate fix just after you.. Ah, I didn't look at this patchset yesterday so didn't notice that Darrick had already posted it. I'm happy for this version to be merged... Reviewed-by: Dave Chinner <dchinner@redhat.com>
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c index d6cd83317344..938023dd8ce5 100644 --- a/fs/xfs/xfs_qm.c +++ b/fs/xfs/xfs_qm.c @@ -148,6 +148,7 @@ xfs_qm_dqpurge( error = xfs_bwrite(bp); xfs_buf_relse(bp); } else if (error == -EAGAIN) { + dqp->dq_flags &= ~XFS_DQ_FREEING; goto out_unlock; } xfs_dqflock(dqp);