From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 06/28] xfs: factor common AIL item deletion code
Date: Mon, 4 Nov 2019 15:16:32 -0800 [thread overview]
Message-ID: <20191104231632.GP4153244@magnolia> (raw)
In-Reply-To: <20191031234618.15403-7-david@fromorbit.com>
On Fri, Nov 01, 2019 at 10:45:56AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Factor the common AIL deletion code that does all the wakeups into a
> helper so we only have one copy of this somewhat tricky code to
> interface with all the wakeups necessary when the LSN of the log
> tail changes.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_inode_item.c | 12 +----------
> fs/xfs/xfs_trans_ail.c | 48 ++++++++++++++++++++++-------------------
> fs/xfs/xfs_trans_priv.h | 4 +++-
> 3 files changed, 30 insertions(+), 34 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index bb8f076805b9..ab12e526540a 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -743,17 +743,7 @@ xfs_iflush_done(
> xfs_clear_li_failed(blip);
> }
> }
> -
> - if (mlip_changed) {
> - if (!XFS_FORCED_SHUTDOWN(ailp->ail_mount))
> - xlog_assign_tail_lsn_locked(ailp->ail_mount);
> - if (list_empty(&ailp->ail_head))
> - wake_up_all(&ailp->ail_empty);
> - }
> - spin_unlock(&ailp->ail_lock);
> -
> - if (mlip_changed)
> - xfs_log_space_wake(ailp->ail_mount);
> + xfs_ail_update_finish(ailp, mlip_changed);
> }
>
> /*
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 6ccfd75d3c24..656819523bbd 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -678,6 +678,27 @@ xfs_ail_push_all_sync(
> finish_wait(&ailp->ail_empty, &wait);
> }
>
> +void
> +xfs_ail_update_finish(
> + struct xfs_ail *ailp,
> + bool do_tail_update) __releases(ailp->ail_lock)
> +{
> + struct xfs_mount *mp = ailp->ail_mount;
> +
> + if (!do_tail_update) {
> + spin_unlock(&ailp->ail_lock);
> + return;
> + }
> +
> + if (!XFS_FORCED_SHUTDOWN(mp))
> + xlog_assign_tail_lsn_locked(mp);
> +
> + if (list_empty(&ailp->ail_head))
> + wake_up_all(&ailp->ail_empty);
> + spin_unlock(&ailp->ail_lock);
> + xfs_log_space_wake(mp);
> +}
> +
> /*
> * xfs_trans_ail_update - bulk AIL insertion operation.
> *
> @@ -737,15 +758,7 @@ xfs_trans_ail_update_bulk(
> if (!list_empty(&tmp))
> xfs_ail_splice(ailp, cur, &tmp, lsn);
>
> - if (mlip_changed) {
> - if (!XFS_FORCED_SHUTDOWN(ailp->ail_mount))
> - xlog_assign_tail_lsn_locked(ailp->ail_mount);
> - spin_unlock(&ailp->ail_lock);
> -
> - xfs_log_space_wake(ailp->ail_mount);
This call site didn't have a wake_up_all and now it does; is that going
to make a difference? I /think/ the answer is that this function
usually puts things on the AIL so we won't trigger the ail_empty wakeup;
and if the AIL was previously empty and we didn't match any log items
(such that it's still empty) then it's fine to wake up anyone who was
waiting for the ail to clear out?
--D
> - } else {
> - spin_unlock(&ailp->ail_lock);
> - }
> + xfs_ail_update_finish(ailp, mlip_changed);
> }
>
> bool
> @@ -789,10 +802,10 @@ void
> xfs_trans_ail_delete(
> struct xfs_ail *ailp,
> struct xfs_log_item *lip,
> - int shutdown_type) __releases(ailp->ail_lock)
> + int shutdown_type)
> {
> struct xfs_mount *mp = ailp->ail_mount;
> - bool mlip_changed;
> + bool need_update;
>
> if (!test_bit(XFS_LI_IN_AIL, &lip->li_flags)) {
> spin_unlock(&ailp->ail_lock);
> @@ -805,17 +818,8 @@ xfs_trans_ail_delete(
> return;
> }
>
> - mlip_changed = xfs_ail_delete_one(ailp, lip);
> - if (mlip_changed) {
> - if (!XFS_FORCED_SHUTDOWN(mp))
> - xlog_assign_tail_lsn_locked(mp);
> - if (list_empty(&ailp->ail_head))
> - wake_up_all(&ailp->ail_empty);
> - }
> -
> - spin_unlock(&ailp->ail_lock);
> - if (mlip_changed)
> - xfs_log_space_wake(ailp->ail_mount);
> + need_update = xfs_ail_delete_one(ailp, lip);
> + xfs_ail_update_finish(ailp, need_update);
> }
>
> int
> diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h
> index 2e073c1c4614..64ffa746730e 100644
> --- a/fs/xfs/xfs_trans_priv.h
> +++ b/fs/xfs/xfs_trans_priv.h
> @@ -92,8 +92,10 @@ xfs_trans_ail_update(
> }
>
> bool xfs_ail_delete_one(struct xfs_ail *ailp, struct xfs_log_item *lip);
> +void xfs_ail_update_finish(struct xfs_ail *ailp, bool do_tail_update)
> + __releases(ailp->ail_lock);
> void xfs_trans_ail_delete(struct xfs_ail *ailp, struct xfs_log_item *lip,
> - int shutdown_type) __releases(ailp->ail_lock);
> + int shutdown_type);
>
> static inline void
> xfs_trans_ail_remove(
> --
> 2.24.0.rc0
>
next prev parent reply other threads:[~2019-11-04 23:16 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-31 23:45 [PATCH 00/28] mm, xfs: non-blocking inode reclaim Dave Chinner
2019-10-31 23:45 ` [PATCH 01/28] xfs: Lower CIL flush limit for large logs Dave Chinner
2019-10-31 23:45 ` [PATCH 02/28] xfs: Throttle commits on delayed background CIL push Dave Chinner
2019-11-01 12:04 ` Brian Foster
2019-11-01 21:40 ` Dave Chinner
2019-11-04 22:48 ` Darrick J. Wong
2019-10-31 23:45 ` [PATCH 03/28] xfs: don't allow log IO to be throttled Dave Chinner
2019-10-31 23:45 ` [PATCH 04/28] xfs: Improve metadata buffer reclaim accountability Dave Chinner
2019-11-01 12:05 ` Brian Foster
2019-11-04 23:21 ` Darrick J. Wong
2019-10-31 23:45 ` [PATCH 05/28] xfs: correctly acount for reclaimable slabs Dave Chinner
2019-10-31 23:45 ` [PATCH 06/28] xfs: factor common AIL item deletion code Dave Chinner
2019-11-04 23:16 ` Darrick J. Wong [this message]
2019-10-31 23:45 ` [PATCH 07/28] xfs: tail updates only need to occur when LSN changes Dave Chinner
2019-11-04 23:18 ` Darrick J. Wong
2019-10-31 23:45 ` [PATCH 08/28] xfs: factor inode lookup from xfs_ifree_cluster Dave Chinner
2019-11-01 12:05 ` Brian Foster
2019-11-04 23:20 ` Darrick J. Wong
2019-10-31 23:45 ` [PATCH 09/28] mm: directed shrinker work deferral Dave Chinner
2019-11-04 15:25 ` Brian Foster
2019-11-14 20:49 ` Dave Chinner
2019-11-15 17:21 ` Brian Foster
2019-11-18 0:49 ` Dave Chinner
2019-11-19 15:12 ` Brian Foster
2019-10-31 23:46 ` [PATCH 10/28] shrinkers: use defer_work for GFP_NOFS sensitive shrinkers Dave Chinner
2019-10-31 23:46 ` [PATCH 11/28] mm: factor shrinker work calculations Dave Chinner
2019-11-02 10:55 ` kbuild test robot
2019-11-04 15:29 ` Brian Foster
2019-11-14 20:59 ` Dave Chinner
2019-10-31 23:46 ` [PATCH 12/28] shrinker: defer work only to kswapd Dave Chinner
2019-11-04 15:29 ` Brian Foster
2019-11-14 21:11 ` Dave Chinner
2019-11-15 17:23 ` Brian Foster
2019-10-31 23:46 ` [PATCH 13/28] shrinker: clean up variable types and tracepoints Dave Chinner
2019-11-04 15:30 ` Brian Foster
2019-10-31 23:46 ` [PATCH 14/28] mm: reclaim_state records pages reclaimed, not slabs Dave Chinner
2019-11-04 19:58 ` Brian Foster
2019-10-31 23:46 ` [PATCH 15/28] mm: back off direct reclaim on excessive shrinker deferral Dave Chinner
2019-11-04 19:58 ` Brian Foster
2019-11-14 21:28 ` Dave Chinner
2019-10-31 23:46 ` [PATCH 16/28] mm: kswapd backoff for shrinkers Dave Chinner
2019-11-04 19:58 ` Brian Foster
2019-11-14 21:41 ` Dave Chinner
2019-10-31 23:46 ` [PATCH 17/28] xfs: synchronous AIL pushing Dave Chinner
2019-11-05 17:05 ` Brian Foster
2019-10-31 23:46 ` [PATCH 18/28] xfs: don't block kswapd in inode reclaim Dave Chinner
2019-10-31 23:46 ` [PATCH 19/28] xfs: reduce kswapd blocking on inode locking Dave Chinner
2019-11-05 17:05 ` Brian Foster
2019-10-31 23:46 ` [PATCH 20/28] xfs: kill background reclaim work Dave Chinner
2019-11-05 17:05 ` Brian Foster
2019-10-31 23:46 ` [PATCH 21/28] xfs: use AIL pushing for inode reclaim IO Dave Chinner
2019-11-05 17:06 ` Brian Foster
2019-10-31 23:46 ` [PATCH 22/28] xfs: remove mode from xfs_reclaim_inodes() Dave Chinner
2019-10-31 23:46 ` [PATCH 23/28] xfs: track reclaimable inodes using a LRU list Dave Chinner
2019-10-31 23:46 ` [PATCH 24/28] xfs: reclaim inodes from the LRU Dave Chinner
2019-11-06 17:21 ` Brian Foster
2019-11-14 21:51 ` Dave Chinner
2019-10-31 23:46 ` [PATCH 25/28] xfs: remove unusued old inode reclaim code Dave Chinner
2019-11-06 17:21 ` Brian Foster
2019-10-31 23:46 ` [PATCH 26/28] xfs: use xfs_ail_push_all in xfs_reclaim_inodes Dave Chinner
2019-11-06 17:22 ` Brian Foster
2019-11-14 21:53 ` Dave Chinner
2019-10-31 23:46 ` [PATCH 27/28] rwsem: introduce down/up_write_non_owner Dave Chinner
2019-10-31 23:46 ` [PATCH 28/28] xfs: rework unreferenced inode lookups Dave Chinner
2019-11-06 22:18 ` Brian Foster
2019-11-14 22:16 ` Dave Chinner
2019-11-15 13:13 ` Christoph Hellwig
2019-11-15 17:26 ` Brian Foster
2019-11-18 1:00 ` Dave Chinner
2019-11-19 15:13 ` Brian Foster
2019-11-19 21:18 ` Dave Chinner
2019-11-20 12:42 ` Brian Foster
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=20191104231632.GP4153244@magnolia \
--to=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--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).