On Wed, Sep 16, 2020 at 08:29:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> The defer ops code has been finishing items in the wrong order -- if a
<snip long explanation>
Yeah, I'd kinda come to the same conclusion while reviewing the
recovery process. The analogy I made in my mind was the difference
in overhead of tracking a breadth-first tree walk vs a depth-first
tree walk...
> As originally written, the code used list_splice_tail_init instead of
> list_splice_init, so change that, and leave a short comment explaining
> our actions.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_defer.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 97523b394932..84a70edd0da1 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -431,8 +431,17 @@ xfs_defer_finish_noroll(
>
> /* Until we run out of pending work to finish... */
> while (!list_empty(&dop_pending) || !list_empty(&(*tp)->t_dfops)) {
> + /*
> + * Deferred items that are created in the process of finishing
> + * other deferred work items should be queued at the head of
> + * the pending list, which puts them ahead of the deferred work
> + * that was created by the caller. This keeps the number of
> + * pending work items to a minimum, which decreases the amount
> + * of time that any one intent item can stick around in memory,
> + * pinning the log tail.
> + */
> xfs_defer_create_intents(*tp);
> - list_splice_tail_init(&(*tp)->t_dfops, &dop_pending);
> + list_splice_init(&(*tp)->t_dfops, &dop_pending);
*nod*.
My favourite sort of fix - a couple of hundred lines of explanation
for a one-liner :)
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
