From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Dave Chinner <david@fromorbit.com>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/11] xfs: deferred inode inactivation
Date: Wed, 24 Mar 2021 17:53:11 +0000 [thread overview]
Message-ID: <20210324175311.GA2773443@infradead.org> (raw)
In-Reply-To: <20210323040037.GI22100@magnolia>
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.
next prev parent reply other threads:[~2021-03-24 17:54 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-11 3:05 [PATCHSET v3 00/11] xfs: deferred inode inactivation Darrick J. Wong
2021-03-11 3:05 ` [PATCH 01/11] xfs: prevent metadata files from being inactivated Darrick J. Wong
2021-03-11 13:05 ` Christoph Hellwig
2021-03-22 23:13 ` Dave Chinner
2021-03-11 3:05 ` [PATCH 02/11] xfs: refactor the predicate part of xfs_free_eofblocks Darrick J. Wong
2021-03-11 13:09 ` Christoph Hellwig
2021-03-15 18:46 ` Christoph Hellwig
2021-03-18 4:33 ` Darrick J. Wong
2021-03-19 1:48 ` Darrick J. Wong
2021-03-11 3:05 ` [PATCH 03/11] xfs: don't reclaim dquots with incore reservations Darrick J. Wong
2021-03-15 18:29 ` Christoph Hellwig
2021-03-22 23:31 ` Dave Chinner
2021-03-23 0:01 ` Darrick J. Wong
2021-03-23 1:48 ` Dave Chinner
2021-03-11 3:06 ` [PATCH 04/11] xfs: decide if inode needs inactivation Darrick J. Wong
2021-03-15 18:47 ` Christoph Hellwig
2021-03-15 19:06 ` Darrick J. Wong
2021-03-11 3:06 ` [PATCH 05/11] xfs: rename the blockgc workqueue Darrick J. Wong
2021-03-15 18:49 ` Christoph Hellwig
2021-03-11 3:06 ` [PATCH 06/11] xfs: deferred inode inactivation Darrick J. Wong
2021-03-16 7:27 ` Christoph Hellwig
2021-03-16 15:47 ` Darrick J. Wong
2021-03-17 15:21 ` Christoph Hellwig
2021-03-17 15:49 ` Darrick J. Wong
2021-03-22 23:46 ` Dave Chinner
2021-03-22 23:37 ` Dave Chinner
2021-03-23 0:24 ` Darrick J. Wong
2021-03-23 1:44 ` Dave Chinner
2021-03-23 4:00 ` Darrick J. Wong
2021-03-23 5:19 ` Dave Chinner
2021-03-24 2:04 ` Darrick J. Wong
2021-03-24 4:57 ` Dave Chinner
2021-03-25 4:20 ` Darrick J. Wong
2021-03-24 17:53 ` Christoph Hellwig [this message]
2021-03-25 4:26 ` Darrick J. Wong
2021-03-11 3:06 ` [PATCH 07/11] xfs: expose sysfs knob to control inode inactivation delay Darrick J. Wong
2021-03-11 3:06 ` [PATCH 08/11] xfs: force inode inactivation and retry fs writes when there isn't space Darrick J. Wong
2021-03-15 18:54 ` Christoph Hellwig
2021-03-15 19:06 ` Darrick J. Wong
2021-03-11 3:06 ` [PATCH 09/11] xfs: force inode garbage collection before fallocate when space is low Darrick J. Wong
2021-03-11 3:06 ` [PATCH 10/11] xfs: parallelize inode inactivation Darrick J. Wong
2021-03-15 18:55 ` Christoph Hellwig
2021-03-15 19:03 ` Darrick J. Wong
2021-03-23 22:21 ` Dave Chinner
2021-03-24 3:52 ` Darrick J. Wong
2021-03-11 3:06 ` [PATCH 11/11] xfs: create a polled function to force " Darrick J. Wong
2021-03-23 22:31 ` Dave Chinner
2021-03-24 3:34 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210324175311.GA2773443@infradead.org \
--to=hch@infradead.org \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).