| Message ID | 20240124204909.105952-10-oliver.upton@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Improvements to GICv3 LPI injection | expand |
On Wed, 24 Jan 2024 20:49:03 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > It will soon be possible for get() and put() calls to happen in > parallel, which means in most cases we must ensure the refcount is > nonzero when taking a new reference. Switch to using > vgic_try_get_irq_kref() where necessary, and document the few conditions > where an IRQ's refcount is guaranteed to be nonzero. > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > --- > arch/arm64/kvm/vgic/vgic-its.c | 18 ++++++++---------- > arch/arm64/kvm/vgic/vgic.c | 3 ++- > 2 files changed, 10 insertions(+), 11 deletions(-) > > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c > index 1d912a595b71..7219f4a0a93d 100644 > --- a/arch/arm64/kvm/vgic/vgic-its.c > +++ b/arch/arm64/kvm/vgic/vgic-its.c > @@ -75,18 +75,11 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid, > * check that we don't add a second list entry with the same LPI. > */ > oldirq = xa_load(&dist->lpi_xa, intid); > - if (oldirq) { > + if (oldirq && vgic_try_get_irq_kref(oldirq)) { nit: move the NULL-check inside the helper, since you always have to do it. Thanks, M.
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c index 1d912a595b71..7219f4a0a93d 100644 --- a/arch/arm64/kvm/vgic/vgic-its.c +++ b/arch/arm64/kvm/vgic/vgic-its.c @@ -75,18 +75,11 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid, * check that we don't add a second list entry with the same LPI. */ oldirq = xa_load(&dist->lpi_xa, intid); - if (oldirq) { + if (oldirq && vgic_try_get_irq_kref(oldirq)) { /* Someone was faster with adding this LPI, lets use that. */ kfree(irq); irq = oldirq; - /* - * This increases the refcount, the caller is expected to - * call vgic_put_irq() on the returned pointer once it's - * finished with the IRQ. - */ - vgic_get_irq_kref(irq); - goto out_unlock; } @@ -607,8 +600,8 @@ static struct vgic_irq *vgic_its_check_cache(struct kvm *kvm, phys_addr_t db, raw_spin_lock_irqsave(&dist->lpi_list_lock, flags); irq = __vgic_its_check_cache(dist, db, devid, eventid); - if (irq) - vgic_get_irq_kref(irq); + if (irq && !vgic_try_get_irq_kref(irq)) + irq = NULL; raw_spin_unlock_irqrestore(&dist->lpi_list_lock, flags); @@ -654,6 +647,11 @@ static void vgic_its_cache_translation(struct kvm *kvm, struct vgic_its *its, if (cte->irq) __vgic_put_lpi_locked(kvm, cte->irq); + /* + * The irq refcount is guaranteed to be nonzero while holding the + * its_lock, as the ITE (and the reference it holds) cannot be freed. + */ + lockdep_assert_held(&its->its_lock); vgic_get_irq_kref(irq); cte->db = db; diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c index 949af87bb599..7e0d84906a12 100644 --- a/arch/arm64/kvm/vgic/vgic.c +++ b/arch/arm64/kvm/vgic/vgic.c @@ -394,7 +394,8 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq, /* * Grab a reference to the irq to reflect the fact that it is - * now in the ap_list. + * now in the ap_list. This is safe as the caller must already hold a + * reference on the irq. */ vgic_get_irq_kref(irq); list_add_tail(&irq->ap_list, &vcpu->arch.vgic_cpu.ap_list_head);
It will soon be possible for get() and put() calls to happen in parallel, which means in most cases we must ensure the refcount is nonzero when taking a new reference. Switch to using vgic_try_get_irq_kref() where necessary, and document the few conditions where an IRQ's refcount is guaranteed to be nonzero. Signed-off-by: Oliver Upton <oliver.upton@linux.dev> --- arch/arm64/kvm/vgic/vgic-its.c | 18 ++++++++---------- arch/arm64/kvm/vgic/vgic.c | 3 ++- 2 files changed, 10 insertions(+), 11 deletions(-)