On Tue, 13 Feb 2024 09:32:44 +0000, Oliver Upton <oliver.upton@xxxxxxxxx> wrote: > > Switch to using atomics for LPI accounting, allowing vgic_irq references > to be dropped in parallel. > > Signed-off-by: Oliver Upton <oliver.upton@xxxxxxxxx> > --- > arch/arm64/kvm/vgic/vgic-debug.c | 2 +- > arch/arm64/kvm/vgic/vgic-its.c | 4 ++-- > arch/arm64/kvm/vgic/vgic.c | 2 +- > include/kvm/arm_vgic.h | 4 ++-- > 4 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/kvm/vgic/vgic-debug.c b/arch/arm64/kvm/vgic/vgic-debug.c > index 85606a531dc3..389025ce7749 100644 > --- a/arch/arm64/kvm/vgic/vgic-debug.c > +++ b/arch/arm64/kvm/vgic/vgic-debug.c > @@ -149,7 +149,7 @@ static void print_dist_state(struct seq_file *s, struct vgic_dist *dist) > seq_printf(s, "vgic_model:\t%s\n", v3 ? "GICv3" : "GICv2"); > seq_printf(s, "nr_spis:\t%d\n", dist->nr_spis); > if (v3) > - seq_printf(s, "nr_lpis:\t%d\n", dist->lpi_list_count); > + seq_printf(s, "nr_lpis:\t%d\n", atomic_read(&dist->lpi_count)); > seq_printf(s, "enabled:\t%d\n", dist->enabled); > seq_printf(s, "\n"); > > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c > index c68164d6cba0..048226812974 100644 > --- a/arch/arm64/kvm/vgic/vgic-its.c > +++ b/arch/arm64/kvm/vgic/vgic-its.c > @@ -97,7 +97,7 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid, > goto out_unlock; > } > > - dist->lpi_list_count++; > + atomic_inc(&dist->lpi_count); > > out_unlock: > if (ret) > @@ -345,7 +345,7 @@ int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr) > * command). If coming from another path (such as enabling LPIs), > * we must be careful not to overrun the array. > */ > - irq_count = READ_ONCE(dist->lpi_list_count); > + irq_count = atomic_read(&dist->lpi_count); I'd like to propose an alternative approach here. I've always hated this "copy a bunch of INTIDs" thing, and the only purpose of this silly counter is to dimension the resulting array. Could we instead rely on an xarray marking a bunch of entries (the ones we want to 'copy'), and get the reader to clear these marks once done? Of course, we only have 3 marks, so that's a bit restrictive from a concurrency perspective, but since most callers hold a lock, it should be OK. What do you think? M. -- Without deviation from the norm, progress is not possible.
