From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/11] xfs: deferred inode inactivation
Date: Wed, 24 Mar 2021 15:57:06 +1100 [thread overview]
Message-ID: <20210324045706.GL63242@dread.disaster.area> (raw)
In-Reply-To: <20210324020407.GO22100@magnolia>
On Tue, Mar 23, 2021 at 07:04:07PM -0700, Darrick J. Wong wrote:
> On Tue, Mar 23, 2021 at 04:19:07PM +1100, Dave Chinner wrote:
> > On Mon, Mar 22, 2021 at 09:00:37PM -0700, Darrick J. Wong wrote:
> > > On Tue, Mar 23, 2021 at 12:44:17PM +1100, Dave Chinner wrote:
> > > > On Wed, Mar 10, 2021 at 07:06:13PM -0800, Darrick J. Wong wrote:
> > > > > + /*
> > > > > + * Not a match for our passed in scan filter? Put it back on the shelf
> > > > > + * and move on.
> > > > > + */
> > > > > + spin_lock(&ip->i_flags_lock);
> > > > > + if (!xfs_inode_matches_eofb(ip, eofb)) {
> > > > > + ip->i_flags &= ~XFS_INACTIVATING;
> > > > > + spin_unlock(&ip->i_flags_lock);
> > > > > + return 0;
> > > > > + }
> > > > > + spin_unlock(&ip->i_flags_lock);
> > > >
> > > > IDGI. What do EOF blocks have to do with running inode inactivation
> > > > on this inode?
> > >
> > > This enables foreground threads that hit EDQUOT to look for inodes to
> > > inactivate in order to free up quota'd resources.
> >
> > Not very obvious - better comment, please?
>
> /*
> * Foreground threads that have hit ENOSPC or EDQUOT are allowed
> * to pass in a eofb structure to look for inodes to inactivate
> * immediately to free some resources. If this inode isn't a
> * match, put it back on the shelf and move on.
> */
>
> Better?
Yes.
> > > > > + /*
> > > > > + * Perform all on-disk metadata updates required to inactivate inodes.
> > > > > + * Since this can involve finobt updates, do it now before we lose the
> > > > > + * per-AG space reservations.
> > > > > + */
> > > > > + xfs_inodegc_force(mp);
> > > >
> > > > Should we stop background inactivation, because we can't make
> > > > modifications anymore and hence background inactication makes little
> > > > sense...
> > >
> > > We don't actually stop background gc transactions or other internal
> > > updates on readonly filesystems
> >
> > Yes we do - that's what xfs_blockgc_stop() higher up in this
> > function does. xfs_log_clean() further down in the function also
> > stops the background log work (that covers the log when idle)
> > because xfs_remount_ro() leaves the log clean.
> >
> > THese all get restarted in xfs_remount_rw()....
> >
> > > -- 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.
> >
> > How do you unlink the file on a RO mount?
>
> I got confused here. If you open a file readonly on a rw mount, unlink
> it, remount the fs readonly, and close the file, we'll still free it.
Not even that way. :)
You can't remount-ro while there are open-but-unlinked files. See
sb->s_remove_count. It's incremented when drop_link() drops the link
count to zero in the unlink() syscall, then decremented when
__destroy_inode() is called during inode eviction when the final
reference goes away. Hence while we have open but unlinked inodes in
active use, that superblock counter is non-zero.
In sb_prepare_remount_readonly() we have:
if (atomic_long_read(&sb->s_remove_count))
return -EBUSY;
So a remount-ro will fail with -EBUSY while there are open but
unlinked files.
Except, of course, if you are doing an emergency remount-ro from
sysrq, in which case these open-but-unlinked checks are not done,
but when we are forcing the fs to be read-only this way, it's not
being done for correctness (i.e the system is about to be shot down)
so we don't really care...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2021-03-24 4:58 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 [this message]
2021-03-25 4:20 ` Darrick J. Wong
2021-03-24 17:53 ` Christoph Hellwig
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=20210324045706.GL63242@dread.disaster.area \
--to=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).