[PATCH] xfs: restore pre flushing optimization xfsaild iteration behavior

[PATCH] xfs: restore pre flushing optimization xfsaild iteration behavior

[Brian Foster]

Commit f3f7ae68a4ea ("xfs: skip flushing log items during push")
implements an optimization to avoid repeated indirect ->iop_push()
function calls for inodes that are already flushed via cluster
buffer flushes. It also changes how flushing inodes are accounted
and how xfsaild processes the AIL list.

We have a report of a soft lockup in this task that is potentially
enabled by these behavior changes. The schedule timeout for the
partial push case was reduced to 0 and the flushing inode item
filtering changes the semantics of the 90% stuck heuristic, which
potentially bypasses another form of task throttling. Both of these
combined increase the risk of excessive spinning.

To address this problem, rework the original optimization to skip
->iop_push() calls without altering iteration and accounting
behavior. Push the flushing check down into xfsaild_push_item(),
which allows flushing inodes to be accounted as they were prior to
the original commit. Remove the count based loop iteration break and
restore the previous tout value. These changes can be revisited in a
follow on patch that documents design/intent and includes any newly
discovered dependent functionality.

Reported-by: MingTao Huang <mintaohuang@tencent.com>
Assisted-by: LLM
Fixes: f3f7ae68a4ea ("xfs: skip flushing log items during push")
Signed-off-by: Brian Foster <bfoster@redhat.com>
---

This is completely untested. I'm mainly posting as a point of reference
for the discussion here [1], and there is disagreement. I'll revisit in
the future if this becomes worth pushing further.

Brian

[1] https://lore.kernel.org/linux-xfs/tencent_F801A9436DFB447AAFEC63D3E615B8052405@qq.com/

 fs/xfs/xfs_trans_ail.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index f955479a08fd..652b09c7dce3 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -392,6 +392,8 @@ xfsaild_push_item(
 	 */
 	if (!lip->li_ops->iop_push)
 		return XFS_ITEM_PINNED;
+	if (test_bit(XFS_LI_FLUSHING, &lip->li_flags))
+		return XFS_ITEM_FLUSHING;
 	if (test_bit(XFS_LI_FAILED, &lip->li_flags))
 		return xfsaild_resubmit_item(lip, &ailp->ail_buf_list);
 	return lip->li_ops->iop_push(lip, &ailp->ail_buf_list);
@@ -581,9 +583,6 @@ 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))
-			goto next_item;
-
 		xfsaild_process_logitem(ailp, lip, &stuck, &flushing);
 		count++;
 
@@ -603,12 +602,9 @@ xfsaild_push(
 		if (stuck > 100)
 			break;
 
-next_item:
 		lip = xfs_trans_ail_cursor_next(ailp, &cur);
 		if (lip == NULL)
 			break;
-		if (lip->li_lsn != lsn && count > 1000)
-			break;
 		lsn = lip->li_lsn;
 	}
 
@@ -645,7 +641,7 @@ xfsaild_push(
 		/*
 		 * Assume we have more work to do in a short while.
 		 */
-		tout = 0;
+		tout = 10;
 	}
 
 	return tout;
-- 
2.55.0