From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/12] xfs: refactor walking of per-AG RECLAIM inodes
Date: Fri, 11 Jan 2019 14:06:06 -0500 [thread overview]
Message-ID: <20190111190606.GC33275@bfoster> (raw)
In-Reply-To: <154630904767.16693.13857289605746264694.stgit@magnolia>
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.
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-11 19:06 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 [this message]
2019-01-17 17:43 ` Darrick J. Wong
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=20190111190606.GC33275@bfoster \
--to=bfoster@redhat.com \
--cc=darrick.wong@oracle.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).