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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.