All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: MingTao Huang <1037827920@qq.com>
Cc: Carlos Maiolino <cem@kernel.org>,
	Dave Chinner <dchinner@redhat.com>,
	Chandan Babu R <chandanbabu@kernel.org>,
	linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	MingTao Huang <mintaohuang@tencent.com>
Subject: Re: [PATCH] xfs: fix skipped flushing items not counted in xfsaild_push()
Date: Tue, 22 Sep 2026 08:02:40 -0700	[thread overview]
Message-ID: <20260922150240.GQ2705364@frogsfrogsfrogs> (raw)
In-Reply-To: <tencent_F801A9436DFB447AAFEC63D3E615B8052405@qq.com>

On Tue, Sep 22, 2026 at 04:14:29PM +0800, MingTao Huang wrote:
> From: MingTao Huang <mintaohuang@tencent.com>
> 
> Commit f3f7ae68a4ea ("xfs: skip flushing log items during push")
> introduced a fast path in xfsaild_push() that uses
> test_bit(XFS_LI_FLUSHING) to skip log items already being
> flushed.  However, the fast path jumps directly to the
> next_item label, bypassing flushing++, count++, and the
> ail_last_pushed_lsn update.  This causes three problems:
> 
> 1. The loop exit condition "count > 1000" becomes much harder to
>    trigger.  count was meant to track every item visited, but now it
>    only increments for non-flushing items that enter
>    xfsaild_push_item().  Each such item typically triggers an inode
>    cluster flush that marks dozens of neighbouring inodes as flushing,
>    so count effectively counts cluster flushes rather than individual
>    items.  The threshold shifts from 1000 items to ~1000 clusters,
>    letting the loop scan an order of magnitude more items per round.
>    Each cluster flush adds a buffer to ail_buf_list, and the resulting
>    oversized list causes xfs_buf_delwri_submit_nowait() -- which runs
>    list_sort() plus per-buffer trylock and IO submission -- to take so
>    long that the watchdog fires.
> 
> 2. The timeout decision "(stuck + flushing) * 100 / count > 90" is
>    computed without the fast-path flushing items, so the flushing ratio
>    is severely under-reported.  When most of the AIL is flushing, the
>    ratio appears near 0%.  xfsaild therefore selects
>    "tout = 0" when it should select "tout = 20" (20 ms
>    back-off to let IO complete).  The zero-backoff tight loop compounds
>    the ail_buf_list accumulation across rounds.
> 
> 3. ail_last_pushed_lsn is not advanced past flushing items, so the
>    next push round restarts scanning from the same position, repeatedly
>    traversing items that are still in-flight.
> 
> We hit this as a soft lockup during stress testing on an internal
> kernel that includes commit f3f7ae68a4ea ("xfs: skip flushing log
> items during push").  The xfsaild kthread was stuck for
> 22 seconds inside xfs_buf_delwri_submit_nowait(), called from
> xfsaild_push(), processing an excessively large ail_buf_list:
> 
>   watchdog: BUG: soft lockup - CPU#48 stuck for 22s! [xfsaild/dm-1:4931]
>   RIP: 0010:xfs_buf_delwri_submit_buffers+0xf2/0x250 [xfs]
>   Call Trace:
>    <TASK>
>    xfsaild_push+0x19b/0x7d0 [xfs]
>    xfsaild+0xb8/0x1a0 [xfs]
>    kthread+0xcc/0x100
>    ret_from_fork+0x5f/0xa0
>    ret_from_fork_asm+0x1b/0x30
>    </TASK>
>   Kernel panic - not syncing: softlockup: hung tasks
> 
> Fix this by accounting for flushing items in the fast path -- increment
> flushing and count, and update ail_last_pushed_lsn -- to match what the
> XFS_ITEM_FLUSHING case in xfsaild_push_item() already does.  This

Am I missing something?  xfsaild_push_item in 7.3-rc4 doesn't seem to
handle flushing.  Maybe you meant xfsaild_process_logitem?  In which
case bumping flushing/counted makes sense.  I /think/ bumping
ail_last_pushed_lsn makes sense too, but I want to think about that
more.

<confused>

--D

> ensures the loop exit condition, the timeout ratio, and the resume
> position all reflect the true state of the AIL.
> 
> Fixes: f3f7ae68a4ea ("xfs: skip flushing log items during push")
> Signed-off-by: MingTao Huang <mintaohuang@tencent.com>
> ---
>  fs/xfs/xfs_trans_ail.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 99a9bf3762b7..0f72cd4e6983 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -580,8 +580,12 @@ xfsaild_push(
>  	lsn = lip->li_lsn;
>  	while ((XFS_LSN_CMP(lip->li_lsn, ailp->ail_target) <= 0)) {
>  
> -		if (test_bit(XFS_LI_FLUSHING, &lip->li_flags))
> +		if (test_bit(XFS_LI_FLUSHING, &lip->li_flags)) {
> +			flushing++;
> +			count++;
> +			ailp->ail_last_pushed_lsn = lsn;
>  			goto next_item;
> +		}
>  
>  		xfsaild_process_logitem(ailp, lip, &stuck, &flushing);
>  		count++;
> -- 
> 2.43.7
> 
> 

  reply	other threads:[~2026-09-22 15:02 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22  8:14 [PATCH] xfs: fix skipped flushing items not counted in xfsaild_push() MingTao Huang
2026-09-22 15:02 ` Darrick J. Wong [this message]
2026-09-22 22:12 ` Dave Chinner
2026-09-23 13:44   ` Brian Foster
2026-09-24  0:34     ` Dave Chinner
2026-09-28 18:51       ` Brian Foster
2026-09-29  2:13         ` Dave Chinner
2026-09-30 13:56           ` Brian Foster
2026-10-06  4:19             ` Dave Chinner
2026-10-08 12:33               ` Brian Foster
2026-10-08 12:41               ` Carlos Maiolino
2026-09-24  2:11   ` MingTao Huang

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=20260922150240.GQ2705364@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=1037827920@qq.com \
    --cc=cem@kernel.org \
    --cc=chandanbabu@kernel.org \
    --cc=dchinner@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=mintaohuang@tencent.com \
    /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.