On 22/06/16 09:18, Andre Przywara wrote:

Hi Marc,

On 17/06/16 13:08, Andre Przywara wrote:

quoted

In the moment our struct vgic_irq's are statically allocated at guest
creation time. So getting a pointer to an IRQ structure is trivial and
safe. LPIs are more dynamic, they can be mapped and unmapped at any time
during the guest's _runtime_.
In preparation for supporting LPIs we introduce reference counting to
those structures. Since private IRQs and SPIs are statically allocated,
the reqcount never drops to 0 at the moment, but we increase it when an
IRQ gets onto a VCPU list and decrease it when it gets removed.
Also vgic_get_irq() gets changed to take the irq_lock already, this is
later needed to avoid a race between a potential LPI unmap and the
window between us getting the pointer and locking the IRQ.
This introduces vgic_put_irq(), which just does the unlock and makes
the new locking sequence look more symmetrical.
This approach deviates from the classical Linux refcounting with using
atomic_* types and functions, because the users of vgic_get_irq() take
the irq_lock anyway, so we just use an int and adjust the refcount while
holding the lock.

As discussed verbally, I am almost done on an implementation that uses
the kernel's kref_* infrastructure to implement the ref-counting, on top
of the existing vgic_irq locking.
Since we consider the atomic inc/dec operations comparably cheap, that
shouldn't sacrifice performance, but will increase readability and
avoids nasty bugs in our own refcounting implementation by using well
tested and reviewed code.

Indeed. And for further reference, here's the reason why I'm pushing for
kref being used: https://lwn.net/Articles/75920/

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...