From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 21/24] xfs: rename xfs_iflush_int()
Date: Fri, 22 May 2020 16:57:38 -0700 [thread overview]
Message-ID: <20200522235738.GY8230@magnolia> (raw)
In-Reply-To: <20200522035029.3022405-22-david@fromorbit.com>
On Fri, May 22, 2020 at 01:50:26PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> with xfs_iflush() gone, we can rename xfs_iflush_int() back to
> xfs_iflush(). Also move it up above xfs_iflush_cluster() so we don't
> need the forward definition any more.
So of course git moves xfs_iflush_cluster instead. Why move 114 lines
when you could move 146? :P
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Eh, whatever,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/xfs_inode.c | 293 ++++++++++++++++++++++-----------------------
> 1 file changed, 146 insertions(+), 147 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index a94528d26328b..cbf8edf62d102 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -44,7 +44,6 @@ kmem_zone_t *xfs_inode_zone;
> */
> #define XFS_ITRUNC_MAX_EXTENTS 2
>
> -STATIC int xfs_iflush_int(struct xfs_inode *, struct xfs_buf *);
> STATIC int xfs_iunlink(struct xfs_trans *, struct xfs_inode *);
> STATIC int xfs_iunlink_remove(struct xfs_trans *, struct xfs_inode *);
>
> @@ -3429,152 +3428,8 @@ xfs_rename(
> return error;
> }
>
> -/*
> - * Non-blocking flush of dirty inode metadata into the backing buffer.
> - *
> - * The caller must have a reference to the inode and hold the cluster buffer
> - * locked. The function will walk across all the inodes on the cluster buffer it
> - * can find and lock without blocking, and flush them to the cluster buffer.
> - *
> - * On success, the caller must write out the buffer returned in *bp and
> - * release it. On failure, the filesystem will be shut down, the buffer will
> - * have been unlocked and released, and EFSCORRUPTED will be returned.
> - */
> -int
> -xfs_iflush_cluster(
> - struct xfs_inode *ip,
> - struct xfs_buf *bp)
> -{
> - struct xfs_mount *mp = ip->i_mount;
> - struct xfs_perag *pag;
> - unsigned long first_index, mask;
> - int cilist_size;
> - struct xfs_inode **cilist;
> - struct xfs_inode *cip;
> - struct xfs_ino_geometry *igeo = M_IGEO(mp);
> - int error = 0;
> - int nr_found;
> - int clcount = 0;
> - int i;
> -
> - pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
> -
> - cilist_size = igeo->inodes_per_cluster * sizeof(struct xfs_inode *);
> - cilist = kmem_alloc(cilist_size, KM_MAYFAIL|KM_NOFS);
> - if (!cilist)
> - goto out_put;
> -
> - mask = ~(igeo->inodes_per_cluster - 1);
> - first_index = XFS_INO_TO_AGINO(mp, ip->i_ino) & mask;
> - rcu_read_lock();
> - /* really need a gang lookup range call here */
> - nr_found = radix_tree_gang_lookup(&pag->pag_ici_root, (void**)cilist,
> - first_index, igeo->inodes_per_cluster);
> - if (nr_found == 0)
> - goto out_free;
> -
> - for (i = 0; i < nr_found; i++) {
> - cip = cilist[i];
> -
> - /*
> - * because this is an RCU protected lookup, we could find a
> - * recently freed or even reallocated inode during the lookup.
> - * We need to check under the i_flags_lock for a valid inode
> - * here. Skip it if it is not valid or the wrong inode.
> - */
> - spin_lock(&cip->i_flags_lock);
> - if (!cip->i_ino ||
> - __xfs_iflags_test(cip, XFS_ISTALE)) {
> - spin_unlock(&cip->i_flags_lock);
> - continue;
> - }
> -
> - /*
> - * Once we fall off the end of the cluster, no point checking
> - * any more inodes in the list because they will also all be
> - * outside the cluster.
> - */
> - if ((XFS_INO_TO_AGINO(mp, cip->i_ino) & mask) != first_index) {
> - spin_unlock(&cip->i_flags_lock);
> - break;
> - }
> - spin_unlock(&cip->i_flags_lock);
> -
> - /*
> - * Do an un-protected check to see if the inode is dirty and
> - * is a candidate for flushing. These checks will be repeated
> - * later after the appropriate locks are acquired.
> - */
> - if (xfs_inode_clean(cip) && xfs_ipincount(cip) == 0)
> - continue;
> -
> - /*
> - * Try to get locks. If any are unavailable or it is pinned,
> - * then this inode cannot be flushed and is skipped.
> - */
> -
> - if (!xfs_ilock_nowait(cip, XFS_ILOCK_SHARED))
> - continue;
> - if (!xfs_iflock_nowait(cip)) {
> - xfs_iunlock(cip, XFS_ILOCK_SHARED);
> - continue;
> - }
> - if (xfs_ipincount(cip)) {
> - xfs_ifunlock(cip);
> - xfs_iunlock(cip, XFS_ILOCK_SHARED);
> - continue;
> - }
> -
> -
> - /*
> - * Check the inode number again, just to be certain we are not
> - * racing with freeing in xfs_reclaim_inode(). See the comments
> - * in that function for more information as to why the initial
> - * check is not sufficient.
> - */
> - if (!cip->i_ino) {
> - xfs_ifunlock(cip);
> - xfs_iunlock(cip, XFS_ILOCK_SHARED);
> - continue;
> - }
> -
> - /*
> - * arriving here means that this inode can be flushed. First
> - * re-check that it's dirty before flushing.
> - */
> - if (!xfs_inode_clean(cip)) {
> - error = xfs_iflush_int(cip, bp);
> - if (error) {
> - xfs_iunlock(cip, XFS_ILOCK_SHARED);
> - goto out_free;
> - }
> - clcount++;
> - } else {
> - xfs_ifunlock(cip);
> - }
> - xfs_iunlock(cip, XFS_ILOCK_SHARED);
> - }
> -
> - if (clcount) {
> - XFS_STATS_INC(mp, xs_icluster_flushcnt);
> - XFS_STATS_ADD(mp, xs_icluster_flushinode, clcount);
> - }
> -
> -out_free:
> - rcu_read_unlock();
> - kmem_free(cilist);
> -out_put:
> - xfs_perag_put(pag);
> - if (error) {
> - bp->b_flags |= XBF_ASYNC;
> - xfs_buf_ioend_fail(bp);
> - xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> - }
> - return error;
> -}
> -
> -STATIC int
> -xfs_iflush_int(
> +static int
> +xfs_iflush(
> struct xfs_inode *ip,
> struct xfs_buf *bp)
> {
> @@ -3722,6 +3577,150 @@ xfs_iflush_int(
> return error;
> }
>
> +/*
> + * Non-blocking flush of dirty inode metadata into the backing buffer.
> + *
> + * The caller must have a reference to the inode and hold the cluster buffer
> + * locked. The function will walk across all the inodes on the cluster buffer it
> + * can find and lock without blocking, and flush them to the cluster buffer.
> + *
> + * On success, the caller must write out the buffer returned in *bp and
> + * release it. On failure, the filesystem will be shut down, the buffer will
> + * have been unlocked and released, and EFSCORRUPTED will be returned.
> + */
> +int
> +xfs_iflush_cluster(
> + struct xfs_inode *ip,
> + struct xfs_buf *bp)
> +{
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_perag *pag;
> + unsigned long first_index, mask;
> + int cilist_size;
> + struct xfs_inode **cilist;
> + struct xfs_inode *cip;
> + struct xfs_ino_geometry *igeo = M_IGEO(mp);
> + int error = 0;
> + int nr_found;
> + int clcount = 0;
> + int i;
> +
> + pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
> +
> + cilist_size = igeo->inodes_per_cluster * sizeof(struct xfs_inode *);
> + cilist = kmem_alloc(cilist_size, KM_MAYFAIL|KM_NOFS);
> + if (!cilist)
> + goto out_put;
> +
> + mask = ~(igeo->inodes_per_cluster - 1);
> + first_index = XFS_INO_TO_AGINO(mp, ip->i_ino) & mask;
> + rcu_read_lock();
> + /* really need a gang lookup range call here */
> + nr_found = radix_tree_gang_lookup(&pag->pag_ici_root, (void**)cilist,
> + first_index, igeo->inodes_per_cluster);
> + if (nr_found == 0)
> + goto out_free;
> +
> + for (i = 0; i < nr_found; i++) {
> + cip = cilist[i];
> +
> + /*
> + * because this is an RCU protected lookup, we could find a
> + * recently freed or even reallocated inode during the lookup.
> + * We need to check under the i_flags_lock for a valid inode
> + * here. Skip it if it is not valid or the wrong inode.
> + */
> + spin_lock(&cip->i_flags_lock);
> + if (!cip->i_ino ||
> + __xfs_iflags_test(cip, XFS_ISTALE)) {
> + spin_unlock(&cip->i_flags_lock);
> + continue;
> + }
> +
> + /*
> + * Once we fall off the end of the cluster, no point checking
> + * any more inodes in the list because they will also all be
> + * outside the cluster.
> + */
> + if ((XFS_INO_TO_AGINO(mp, cip->i_ino) & mask) != first_index) {
> + spin_unlock(&cip->i_flags_lock);
> + break;
> + }
> + spin_unlock(&cip->i_flags_lock);
> +
> + /*
> + * Do an un-protected check to see if the inode is dirty and
> + * is a candidate for flushing. These checks will be repeated
> + * later after the appropriate locks are acquired.
> + */
> + if (xfs_inode_clean(cip) && xfs_ipincount(cip) == 0)
> + continue;
> +
> + /*
> + * Try to get locks. If any are unavailable or it is pinned,
> + * then this inode cannot be flushed and is skipped.
> + */
> +
> + if (!xfs_ilock_nowait(cip, XFS_ILOCK_SHARED))
> + continue;
> + if (!xfs_iflock_nowait(cip)) {
> + xfs_iunlock(cip, XFS_ILOCK_SHARED);
> + continue;
> + }
> + if (xfs_ipincount(cip)) {
> + xfs_ifunlock(cip);
> + xfs_iunlock(cip, XFS_ILOCK_SHARED);
> + continue;
> + }
> +
> +
> + /*
> + * Check the inode number again, just to be certain we are not
> + * racing with freeing in xfs_reclaim_inode(). See the comments
> + * in that function for more information as to why the initial
> + * check is not sufficient.
> + */
> + if (!cip->i_ino) {
> + xfs_ifunlock(cip);
> + xfs_iunlock(cip, XFS_ILOCK_SHARED);
> + continue;
> + }
> +
> + /*
> + * arriving here means that this inode can be flushed. First
> + * re-check that it's dirty before flushing.
> + */
> + if (!xfs_inode_clean(cip)) {
> + error = xfs_iflush(cip, bp);
> + if (error) {
> + xfs_iunlock(cip, XFS_ILOCK_SHARED);
> + goto out_free;
> + }
> + clcount++;
> + } else {
> + xfs_ifunlock(cip);
> + }
> + xfs_iunlock(cip, XFS_ILOCK_SHARED);
> + }
> +
> + if (clcount) {
> + XFS_STATS_INC(mp, xs_icluster_flushcnt);
> + XFS_STATS_ADD(mp, xs_icluster_flushinode, clcount);
> + }
> +
> +out_free:
> + rcu_read_unlock();
> + kmem_free(cilist);
> +out_put:
> + xfs_perag_put(pag);
> + if (error) {
> + bp->b_flags |= XBF_ASYNC;
> + xfs_buf_ioend_fail(bp);
> + xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> + }
> + return error;
> +}
> +
> /* Release an inode. */
> void
> xfs_irele(
> --
> 2.26.2.761.g0e0b3e54be
>
next prev parent reply other threads:[~2020-05-22 23:57 UTC|newest]
Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-22 3:50 [PATCH 00/24] xfs: rework inode flushing to make inode reclaim fully asynchronous Dave Chinner
2020-05-22 3:50 ` [PATCH 01/24] xfs: remove logged flag from inode log item Dave Chinner
2020-05-22 7:25 ` Christoph Hellwig
2020-05-22 21:13 ` Darrick J. Wong
2020-05-22 3:50 ` [PATCH 02/24] xfs: add an inode item lock Dave Chinner
2020-05-22 6:45 ` Amir Goldstein
2020-05-22 21:24 ` Darrick J. Wong
2020-05-23 8:45 ` Christoph Hellwig
2020-05-22 3:50 ` [PATCH 03/24] xfs: mark inode buffers in cache Dave Chinner
2020-05-22 7:45 ` Amir Goldstein
2020-05-22 21:35 ` Darrick J. Wong
2020-05-24 23:41 ` Dave Chinner
2020-05-23 8:48 ` Christoph Hellwig
2020-05-25 0:06 ` Dave Chinner
2020-05-22 3:50 ` [PATCH 04/24] xfs: mark dquot " Dave Chinner
2020-05-22 7:46 ` Amir Goldstein
2020-05-22 21:38 ` Darrick J. Wong
2020-05-22 3:50 ` [PATCH 05/24] xfs: mark log recovery buffers for completion Dave Chinner
2020-05-22 7:41 ` Amir Goldstein
2020-05-24 23:54 ` Dave Chinner
2020-05-22 21:41 ` Darrick J. Wong
2020-05-22 3:50 ` [PATCH 06/24] xfs: call xfs_buf_iodone directly Dave Chinner
2020-05-22 7:56 ` Amir Goldstein
2020-05-22 21:53 ` Darrick J. Wong
2020-05-22 3:50 ` [PATCH 07/24] xfs: clean up whacky buffer log item list reinit Dave Chinner
2020-05-22 22:01 ` Darrick J. Wong
2020-05-23 8:50 ` Christoph Hellwig
2020-05-22 3:50 ` [PATCH 08/24] xfs: fold xfs_istale_done into xfs_iflush_done Dave Chinner
2020-05-22 22:10 ` Darrick J. Wong
2020-05-23 9:12 ` Christoph Hellwig
2020-05-22 3:50 ` [PATCH 09/24] xfs: use direct calls for dquot IO completion Dave Chinner
2020-05-22 22:13 ` Darrick J. Wong
2020-05-23 9:16 ` Christoph Hellwig
2020-05-22 3:50 ` [PATCH 10/24] xfs: clean up the buffer iodone callback functions Dave Chinner
2020-05-22 22:26 ` Darrick J. Wong
2020-05-25 0:37 ` Dave Chinner
2020-05-23 9:19 ` Christoph Hellwig
2020-05-22 3:50 ` [PATCH 11/24] xfs: get rid of log item callbacks Dave Chinner
2020-05-22 22:27 ` Darrick J. Wong
2020-05-23 9:19 ` Christoph Hellwig
2020-05-22 3:50 ` [PATCH 12/24] xfs: pin inode backing buffer to the inode log item Dave Chinner
2020-05-22 22:39 ` Darrick J. Wong
2020-05-23 9:34 ` Christoph Hellwig
2020-05-23 21:43 ` Dave Chinner
2020-05-24 5:31 ` Christoph Hellwig
2020-05-24 23:13 ` Dave Chinner
2020-05-22 3:50 ` [PATCH 13/24] xfs: make inode reclaim almost non-blocking Dave Chinner
2020-05-22 12:19 ` Amir Goldstein
2020-05-22 22:48 ` Darrick J. Wong
2020-05-23 22:29 ` Dave Chinner
2020-05-22 3:50 ` [PATCH 14/24] xfs: remove IO submission from xfs_reclaim_inode() Dave Chinner
2020-05-22 23:06 ` Darrick J. Wong
2020-05-25 3:49 ` Dave Chinner
2020-05-23 9:40 ` Christoph Hellwig
2020-05-23 22:35 ` Dave Chinner
2020-05-22 3:50 ` [PATCH 15/24] xfs: allow multiple reclaimers per AG Dave Chinner
2020-05-22 23:10 ` Darrick J. Wong
2020-05-23 22:35 ` Dave Chinner
2020-05-22 3:50 ` [PATCH 16/24] xfs: don't block inode reclaim on the ILOCK Dave Chinner
2020-05-22 23:11 ` Darrick J. Wong
2020-05-22 3:50 ` [PATCH 17/24] xfs: remove SYNC_TRYLOCK from inode reclaim Dave Chinner
2020-05-22 23:14 ` Darrick J. Wong
2020-05-23 22:42 ` Dave Chinner
2020-05-22 3:50 ` [PATCH 18/24] xfs: clean up inode reclaim comments Dave Chinner
2020-05-22 23:17 ` Darrick J. Wong
2020-05-22 3:50 ` [PATCH 19/24] xfs: attach inodes to the cluster buffer when dirtied Dave Chinner
2020-05-22 23:48 ` Darrick J. Wong
2020-05-23 22:59 ` Dave Chinner
2020-05-22 3:50 ` [PATCH 20/24] xfs: xfs_iflush() is no longer necessary Dave Chinner
2020-05-22 23:54 ` Darrick J. Wong
2020-05-22 3:50 ` [PATCH 21/24] xfs: rename xfs_iflush_int() Dave Chinner
2020-05-22 12:33 ` Amir Goldstein
2020-05-22 23:57 ` Darrick J. Wong [this message]
2020-05-22 3:50 ` [PATCH 22/24] xfs: rework xfs_iflush_cluster() dirty inode iteration Dave Chinner
2020-05-23 0:13 ` Darrick J. Wong
2020-05-23 23:14 ` Dave Chinner
2020-05-23 11:31 ` Christoph Hellwig
2020-05-23 23:23 ` Dave Chinner
2020-05-24 5:32 ` Christoph Hellwig
2020-05-23 11:39 ` Christoph Hellwig
2020-05-22 3:50 ` [PATCH 23/24] xfs: factor xfs_iflush_done Dave Chinner
2020-05-23 0:20 ` Darrick J. Wong
2020-05-23 11:35 ` Christoph Hellwig
2020-05-22 3:50 ` [PATCH 24/24] xfs: remove xfs_inobp_check() Dave Chinner
2020-05-23 0:16 ` Darrick J. Wong
2020-05-23 11:36 ` Christoph Hellwig
2020-05-22 4:04 ` [PATCH 00/24] xfs: rework inode flushing to make inode reclaim fully asynchronous Dave Chinner
2020-05-23 16:18 ` Darrick J. Wong
2020-05-23 21:22 ` Dave Chinner
2020-05-22 6:18 ` Amir Goldstein
2020-05-22 12:01 ` Amir Goldstein
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=20200522235738.GY8230@magnolia \
--to=darrick.wong@oracle.com \
--cc=david@fromorbit.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.