On 2026-02-05 at 07:55:25, Jakub Kicinski (kuba@kernel.org) wrote:

quoted

+		dev_err(rvu->dev,
+			"%s: Failed to roll back vidx2idx vidx=%u\n",
+			__func__, vidx);

At fail2, xa_erase() returns the erased entry on success (non-NULL) and NULL
on failure. The error message is printed when xa_erase() succeeds, which
appears inverted from the intended behavior.

Should the condition check for NULL to detect failure instead?

quoted

+
+fail1:
+	if (xa_insert(&npc_priv.xa_vidx2idx_map, vidx,
+		      xa_mk_value(old_midx), GFP_KERNEL))
+		dev_err(rvu->dev,
+			"%s: Faled to roll back vidx2idx to old_midx=%u, vidx=%u\n",
+			__func__, old_midx, vidx);

This isn't a bug, but there's a typo: "Faled" should be "Failed".

Also, at fail1, the rollback attempts to insert into xa_vidx2idx_map to
restore old_midx. However, looking at the control flow, fail1 is reached when
xa_insert() to xa_vidx2idx_map fails. At that point, xa_vidx2idx_map has
already been erased, so the rollback target is correct.

But fail2 rolls back using xa_erase() on xa_vidx2idx_map, which was just
inserted successfully. The rollback at fail1 should restore xa_idx2vidx_map
instead, since that's the only map modified when fail1 is reached via direct
return.

Could the error path rollback logic be reviewed?

This is getting flagged every time. i dont see any issue in the logic. Am i missing
any thing here.