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@xxxxxxxxx> 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
