On Mon, Mar 22, 2021 at 05:01:11PM -0700, Darrick J. Wong wrote:
> 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@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxx>
> > .....
> > > 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?
Yes Please!
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
