From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Brian Foster <bfoster@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/12] xfs: refactor walking of per-AG RECLAIM inodes
Date: Thu, 17 Jan 2019 09:43:31 -0800 [thread overview]
Message-ID: <20190117174331.GC4424@magnolia> (raw)
In-Reply-To: <20190111190606.GC33275@bfoster>
On Fri, Jan 11, 2019 at 02:06:06PM -0500, Brian Foster wrote:
> On Mon, Dec 31, 2018 at 06:17:27PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Refactor the code that walks reclaim-tagged inodes so that we can reuse
> > the same loop in a subsequent patch.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/xfs_icache.c | 170 +++++++++++++++++++++++++++++----------------------
> > 1 file changed, 97 insertions(+), 73 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > index 36d986087abb..7e031eb6f048 100644
> > --- a/fs/xfs/xfs_icache.c
> > +++ b/fs/xfs/xfs_icache.c
> ...
> > @@ -1223,6 +1223,90 @@ xfs_reclaim_inode(
> > return 0;
> > }
> >
> > +/*
> > + * Walk the RECLAIM tagged inodes in this AG looking for inodes to inactivate.
> > + */
> > +STATIC int
> > +xfs_walk_ag_reclaim_inos(
>
> Nit: how about xfs_reclaim_inodes_pag()? Hm, maybe we can rename
> xfs_reclaim_inodes_ag() to something like __xfs_reclaim_inodes() as
> well.
Ok, will do. This whole section might change dramatically if the incore
inode radix tree becomes an xarray, but we'll see.
--D
> Brian
>
> > + struct xfs_perag *pag,
> > + int sync_flags,
> > + bool (*grab_fn)(struct xfs_inode *ip,
> > + int sync_flags),
> > + int (*execute_fn)(struct xfs_inode *ip,
> > + struct xfs_perag *pag,
> > + int sync_flags),
> > + int *nr_to_scan,
> > + bool *done)
> > +{
> > + struct xfs_mount *mp = pag->pag_mount;
> > + unsigned long first_index = 0;
> > + int nr_found = 0;
> > + int last_error = 0;
> > + int error;
> > +
> > + do {
> > + struct xfs_inode *batch[XFS_LOOKUP_BATCH];
> > + int i;
> > +
> > + rcu_read_lock();
> > + nr_found = radix_tree_gang_lookup_tag(&pag->pag_ici_root,
> > + (void **)batch, first_index, XFS_LOOKUP_BATCH,
> > + XFS_ICI_RECLAIM_TAG);
> > + if (!nr_found) {
> > + *done = true;
> > + rcu_read_unlock();
> > + break;
> > + }
> > +
> > + /*
> > + * Grab the inodes before we drop the lock. if we found
> > + * nothing, nr == 0 and the loop will be skipped.
> > + */
> > + for (i = 0; i < nr_found; i++) {
> > + struct xfs_inode *ip = batch[i];
> > +
> > + if (*done || !grab_fn(ip, sync_flags))
> > + batch[i] = NULL;
> > +
> > + /*
> > + * Update the index for the next lookup. Catch
> > + * overflows into the next AG range which can occur if
> > + * we have inodes in the last block of the AG and we
> > + * are currently pointing to the last inode.
> > + *
> > + * Because we may see inodes that are from the wrong AG
> > + * due to RCU freeing and reallocation, only update the
> > + * index if it lies in this AG. It was a race that lead
> > + * us to see this inode, so another lookup from the
> > + * same index will not find it again.
> > + */
> > + if (XFS_INO_TO_AGNO(mp, ip->i_ino) != pag->pag_agno)
> > + continue;
> > + first_index = XFS_INO_TO_AGINO(mp, ip->i_ino + 1);
> > + if (first_index < XFS_INO_TO_AGINO(mp, ip->i_ino))
> > + *done = true;
> > + }
> > +
> > + /* unlock now we've grabbed the inodes. */
> > + rcu_read_unlock();
> > +
> > + for (i = 0; i < nr_found; i++) {
> > + if (!batch[i])
> > + continue;
> > + error = execute_fn(batch[i], pag, sync_flags);
> > + if (error && last_error != -EFSCORRUPTED)
> > + last_error = error;
> > + }
> > +
> > + *nr_to_scan -= XFS_LOOKUP_BATCH;
> > +
> > + cond_resched();
> > +
> > + } while (nr_found && !*done && *nr_to_scan > 0);
> > +
> > + return last_error;
> > +}
> > +
> > /*
> > * Walk the AGs and reclaim the inodes in them. Even if the filesystem is
> > * corrupted, we still want to try to reclaim all the inodes. If we don't,
> > @@ -1236,8 +1320,8 @@ xfs_reclaim_inodes_ag(
> > int *nr_to_scan)
> > {
> > struct xfs_perag *pag;
> > - int error = 0;
> > int last_error = 0;
> > + int error;
> > xfs_agnumber_t ag;
> > int trylock = flags & SYNC_TRYLOCK;
> > int skipped;
> > @@ -1247,8 +1331,7 @@ xfs_reclaim_inodes_ag(
> > skipped = 0;
> > while ((pag = xfs_perag_get_tag(mp, ag, XFS_ICI_RECLAIM_TAG))) {
> > unsigned long first_index = 0;
> > - int done = 0;
> > - int nr_found = 0;
> > + bool done = false;
> >
> > ag = pag->pag_agno + 1;
> >
> > @@ -1262,70 +1345,11 @@ xfs_reclaim_inodes_ag(
> > } else
> > mutex_lock(&pag->pag_ici_reclaim_lock);
> >
> > - do {
> > - struct xfs_inode *batch[XFS_LOOKUP_BATCH];
> > - int i;
> > -
> > - rcu_read_lock();
> > - nr_found = radix_tree_gang_lookup_tag(
> > - &pag->pag_ici_root,
> > - (void **)batch, first_index,
> > - XFS_LOOKUP_BATCH,
> > - XFS_ICI_RECLAIM_TAG);
> > - if (!nr_found) {
> > - done = 1;
> > - rcu_read_unlock();
> > - break;
> > - }
> > -
> > - /*
> > - * Grab the inodes before we drop the lock. if we found
> > - * nothing, nr == 0 and the loop will be skipped.
> > - */
> > - for (i = 0; i < nr_found; i++) {
> > - struct xfs_inode *ip = batch[i];
> > -
> > - if (done || xfs_reclaim_inode_grab(ip, flags))
> > - batch[i] = NULL;
> > -
> > - /*
> > - * Update the index for the next lookup. Catch
> > - * overflows into the next AG range which can
> > - * occur if we have inodes in the last block of
> > - * the AG and we are currently pointing to the
> > - * last inode.
> > - *
> > - * Because we may see inodes that are from the
> > - * wrong AG due to RCU freeing and
> > - * reallocation, only update the index if it
> > - * lies in this AG. It was a race that lead us
> > - * to see this inode, so another lookup from
> > - * the same index will not find it again.
> > - */
> > - if (XFS_INO_TO_AGNO(mp, ip->i_ino) !=
> > - pag->pag_agno)
> > - continue;
> > - first_index = XFS_INO_TO_AGINO(mp, ip->i_ino + 1);
> > - if (first_index < XFS_INO_TO_AGINO(mp, ip->i_ino))
> > - done = 1;
> > - }
> > -
> > - /* unlock now we've grabbed the inodes. */
> > - rcu_read_unlock();
> > -
> > - for (i = 0; i < nr_found; i++) {
> > - if (!batch[i])
> > - continue;
> > - error = xfs_reclaim_inode(batch[i], pag, flags);
> > - if (error && last_error != -EFSCORRUPTED)
> > - last_error = error;
> > - }
> > -
> > - *nr_to_scan -= XFS_LOOKUP_BATCH;
> > -
> > - cond_resched();
> > -
> > - } while (nr_found && !done && *nr_to_scan > 0);
> > + error = xfs_walk_ag_reclaim_inos(pag, flags,
> > + xfs_reclaim_inode_grab, xfs_reclaim_inode,
> > + nr_to_scan, &done);
> > + if (error && last_error != -EFSCORRUPTED)
> > + last_error = error;
> >
> > if (trylock && !done)
> > pag->pag_ici_reclaim_cursor = first_index;
> >
next prev parent reply other threads:[~2019-01-17 17:43 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-01 2:16 [PATCH 00/12] xfs: deferred inode inactivation Darrick J. Wong
2019-01-01 2:16 ` [PATCH 01/12] xfs: free COW staging extents when freezing filesystem Darrick J. Wong
2019-01-11 16:28 ` Brian Foster
2019-01-17 17:24 ` Darrick J. Wong
2019-01-17 18:14 ` Brian Foster
2019-01-17 20:20 ` Darrick J. Wong
2019-01-01 2:17 ` [PATCH 02/12] xfs: refactor the predicate part of xfs_free_eofblocks Darrick J. Wong
2019-01-11 19:05 ` Brian Foster
2019-01-17 17:33 ` Darrick J. Wong
2019-01-01 2:17 ` [PATCH 03/12] xfs: decide if inode needs inactivation Darrick J. Wong
2019-01-01 2:17 ` [PATCH 04/12] xfs: track unlinked inactive inode fs summary counters Darrick J. Wong
2019-01-01 2:17 ` [PATCH 05/12] xfs: track unlinked inactive inode quota counters Darrick J. Wong
2019-01-01 2:17 ` [PATCH 06/12] xfs: refactor walking of per-AG RECLAIM inodes Darrick J. Wong
2019-01-11 19:06 ` Brian Foster
2019-01-17 17:43 ` Darrick J. Wong [this message]
2019-01-01 2:17 ` [PATCH 07/12] xfs: refactor eofblocks inode match code Darrick J. Wong
2019-01-02 9:50 ` Nikolay Borisov
2019-01-17 18:05 ` Darrick J. Wong
2019-01-01 2:17 ` [PATCH 08/12] xfs: deferred inode inactivation Darrick J. Wong
2019-01-01 2:17 ` [PATCH 09/12] xfs: retry fs writes when there isn't space Darrick J. Wong
2019-01-01 2:17 ` [PATCH 10/12] xfs: force inactivation before fallocate when space is low Darrick J. Wong
2019-01-01 2:17 ` [PATCH 11/12] xfs: parallelize inode inactivation Darrick J. Wong
2019-01-01 2:18 ` [PATCH 12/12] xfs: wait for deferred inactivation when destroying unlinked inodes Darrick J. Wong
2019-01-03 12:46 ` Dave Chinner
2019-01-17 18:41 ` Darrick J. Wong
2019-01-17 22:21 ` Dave Chinner
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=20190117174331.GC4424@magnolia \
--to=darrick.wong@oracle.com \
--cc=bfoster@redhat.com \
--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).