Re: [net-next,v8,09/13] octeontx2-af: npc: cn20k: virtual index support
From: Jakub Kicinski
Date: Tue Feb 24 2026 - 13:23:38 EST
On Tue, 24 Feb 2026 11:50:41 +0530 Ratheesh Kannoth wrote:
> > > +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.
Maybe rewrite the code to be less confusing and so that the LLM
understands it? For one thing the jump labels are supposed to have
meaningful names, not err1/err2/err3, or fail1/fail2
Also perhaps xa_store() could avoid the erase + insert dance?