Re: [PATCH 03/11] xfs: don't reclaim dquots with incore reservations

Darrick J. Wong
2021-03-23T00:01:55Z

On Tue, Mar 23, 2021 at 10:31:39AM +1100, Dave Chinner wrote:
> On Wed, Mar 10, 2021 at 07:05:57PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > If a dquot has an incore reservation that exceeds the ondisk count, it
> > by definition has active incore state and must not be reclaimed. Up to
> > this point every inode with an incore dquot reservation has always
> > retained a reference to the dquot so it was never possible for
> > xfs_qm_dquot_isolate to be called on a dquot with active state and zero
> > refcount, but this will soon change.
> >
> > Deferred inode inactivation is about to reorganize how inodes are
> > inactivated by shunting all that work to a background workqueue. In
> > order to avoid deadlocks with the quotaoff inode scan and reduce overall
> > memory requirements (since inodes can spend a lot of time waiting for
> > inactivation), inactive inodes will drop their dquot references while
> > they're waiting to be inactivated.
> >
> > However, inactive inodes can have delalloc extents in the data fork or
> > any extents in the CoW fork. Either of these contribute to the dquot's
> > incore reservation being larger than the resource count (i.e. they're
> > the reason the dquot still has active incore state), so we cannot allow
> > the dquot to be reclaimed.
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> .....
> > static enum lru_status
> > xfs_qm_dquot_isolate(
> > struct list_head *item,
> > @@ -427,10 +441,15 @@ xfs_qm_dquot_isolate(
> > goto out_miss_busy;
> >
> > /*
> > - * This dquot has acquired a reference in the meantime remove it from
> > - * the freelist and try again.
> > + * Either this dquot has incore reservations or it has acquired a
> > + * reference. Remove it from the freelist and try again.
> > + *
> > + * Inodes tagged for inactivation drop their dquot references to avoid
> > + * deadlocks with quotaoff. If these inodes have delalloc reservations
> > + * in the data fork or any extents in the CoW fork, these contribute
> > + * to the dquot's incore block reservation exceeding the count.
> > */
> > - if (dqp->q_nrefs) {
> > + if (xfs_dquot_has_incore_resv(dqp) || dqp->q_nrefs) {
> > xfs_dqunlock(dqp);
> > XFS_STATS_INC(dqp->q_mount, xs_qm_dqwants);
> >
>
> This means we can have dquots with no references that aren't on
> the free list and aren't actually referenced by any inode, either.
>
> So if we now shut down the filesystem, what frees these dquots?
> Are we relying on xfs_qm_dqpurge_all() to find all these dquots
> and xfs_qm_dqpurge() guaranteeing that they are always cleaned
> and freed?

Yes. Want me to add that to the comment?

--D

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com