[PATCH] ipmi: bt: Stop the unbounded reset loop in error_recovery()

[Jianping Liu <frankljpliu@gmail.com>]

From: Jianping Liu <frankjpliu@tencent.com>

When a BT transaction times out and the retry budget is exhausted,
error_recovery() soft-resets the BMC instead of failing the request if
the status register has been non-zero and bt->seq is small.

The reset path ends in BT_STATE_RESTART, which deliberately keeps
bt->error_retries and bt->seq and restarts the same request.  If the BMC
still does not answer, the next timeout re-enters error_recovery() with
error_retries already past BT_CAP_retries and the same seq, so the reset
branch is taken again.  Nothing in the loop changes either condition,
so the request is never completed.

We hit this on a production machine whose BMC stopped responding while
leaving H2B_ATN set.  The same three lines repeat every 11 seconds (the
5 s request timeout plus the 6 s BT_RESET_DELAY), 27321 times over 3.5
days, without a single "retries left" line:

  ipmi_si dmi-ipmi-si.0: IPMI BT: timeout in XACTION [ H2B ]
  ipmi_si dmi-ipmi-si.0: failed 2 retries, sending error response
  ipmi_si dmi-ipmi-si.0: BT reset (takes 5 secs)

The state machine kept the same curr_msg throughout, with
bt->error_retries at 3086544 against a BT_CAP_retries of 2.

Because the transaction never completes, intf->curr_msg is never
released and every later request piles up on the xmit queue.  Each
queued request holds a reference on its ipmi_user, and neither closing
the device nor ipmi_timeout_handler() cancels locally queued messages,
so the users of exited processes are never freed.  On that machine (a
5.4-based kernel, where struct ipmi_user embeds a srcu_struct and is a
52 KiB vmalloc) this left 552291 ipmi_user objects behind, 27.4 GiB or
93.6% of all vmalloc'd pages, and the machine went into repeated OOM
kills.

Only take the reset branch once, at the moment the retry budget is
exhausted.  If the request still fails after the reset, error_recovery()
is entered with error_retries above BT_CAP_retries, skips the reset and
falls through to force_result(), so the request completes with an error
and the queue can move on.

Fixes: 4d7cbac7c870 ("[PATCH] IPMI: Fix BT long busy")
Signed-off-by: Jianping Liu <frankjpliu@tencent.com>
---
 drivers/char/ipmi/ipmi_bt_sm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c
index f41f78972b9c..a8c181196f71 100644
--- a/drivers/char/ipmi/ipmi_bt_sm.c
+++ b/drivers/char/ipmi/ipmi_bt_sm.c
@@ -419,7 +419,8 @@ static enum si_sm_result error_recovery(struct si_sm_data *bt,
 		dev_err(bt->io->dev, "stuck, try power cycle\n");
 
 	/* this is most likely during insmod */
-	else if (bt->seq <= (unsigned char)(bt->BT_CAP_retries & 0xFF)) {
+	else if (bt->error_retries == bt->BT_CAP_retries &&
+			bt->seq <= (unsigned char)(bt->BT_CAP_retries & 0xFF)) {
 		dev_warn(bt->io->dev, "BT reset (takes 5 secs)\n");
 		bt->state = BT_STATE_RESET1;
 		return SI_SM_CALL_WITHOUT_DELAY;
-- 
2.43.7