Re: [PATCH 11/15] KVM: arm64: vgic-its: Lazily allocate LPI translation cache

Oliver Upton
2024-01-25T15:13:16Z

On Thu, Jan 25, 2024 at 10:19:46AM +0000, Marc Zyngier wrote:
> On Wed, 24 Jan 2024 20:49:05 +0000, Oliver Upton <oliver.upton@linux.dev> wrote:
> > +
> > + /*
> > + * Caching the translation implies having an extra reference
> > + * to the interrupt, so drop the potential reference on what
> > + * was in the cache, and increment it on the new interrupt.
> > + */
> > + if (victim && victim->irq)
> > + vgic_put_irq(kvm, victim->irq);
>
> The games you play with 'victim' are a bit odd. I'd rather have it
> initialised to NULL, and be trusted to have a valid irq if non-NULL.
>
> Is there something special I'm missing?

I pulled some shenanigans use the same cleanup path to free the new
cache entry in the case of a race. At that point the new cache entry
is initialized to 0 and doesn't have a valid pointer to an irq.

I thought this was a fun trick, but in retrospect it just makes it hard
to follow. I'll just explicitly free the new entry in the case of a
detected race and do away with the weirdness.

--
Thanks,
Oliver