Re: [PATCH 06/11] xfs: deferred inode inactivation

Christoph Hellwig
2021-03-24T17:54:30Z

On Mon, Mar 22, 2021 at 09:00:37PM -0700, Darrick J. Wong wrote:
> Hmm, maybe this could maintain an approxiate liar counter and only flush
> inactivation when the liar counter would cause us to be off by more than
> some configurable amount? The fstests that care about free space
> accounting are not going to be happy since they are measured with very
> tight tolerances.

Yes, I think some kind of fuzzy logic instead of the heavy weight flush
on supposedly light weight operations.

> > static void
> > xfs_inode_clear_tag(
> > struct xfs_perag *pag,
> > xfs_ino_t ino,
> > int tag)
> > {
> > struct xfs_mount *mp = pag->pag_mount;
> >
> > lockdep_assert_held(&pag->pag_ici_lock);
> > radix_tree_tag_clear(&pag->pag_ici_root, XFS_INO_TO_AGINO(mp, ino),
> > tag);
> > switch(tag) {
> > case XFS_ICI_INACTIVE_TAG:
> > if (--pag->pag_ici_inactive)
> > return;
> > break;
> > case XFS_ICI_RECLAIM_TAG:
> > if (--pag->pag_ici_reclaim)
> > return;
> > break;
> > default:
> > ASSERT(0);
> > return;
> > }
> >
> > spin_lock(&mp->m_perag_lock);
> > radix_tree_tag_clear(&mp->m_perag_tree, pag->pag_agno, tag);
> > spin_unlock(&mp->m_perag_lock);
> > }
> >
> > As a followup patch? The set tag case looks similarly easy to make
> > generic...
>
> Yeah. At this point I might as well just clean all of this up for the
> next revision of this series, because as I said earlier I had thought
> that you were still working on a second rework of reclaim. Now that I
> know you're not, I'll hack away at this twisty pile too.

If the separate tags aren't going to disappear entirely: it would be nice
to move the counters (or any other duplicated variable) into an array
index by the tax, which would clean the above and similar code even more.

> We don't actually stop background gc transactions or other internal
> updates on readonly filesystems -- the ro part means only that we don't
> let /userspace/ change anything directly. If you open a file readonly,
> unlink it, freeze the fs, and close the file, we'll still free it.

Note that there are two different read-only concepts in Linux:

1) the read-only mount, as reflected in the vfsmount. For this your
description above is spot-on
2) the read-only superblock, as indicated by the sb flag. This is
usually due to an read-only block device, and we must not write
anything to the device, as that typically will lead to an I/O error.