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 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).