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

[Oliver Upton <oliver.upton@linux.dev>]

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