On 04/08/15 18:36, Christoffer Dall wrote:
> On Tue, Aug 04, 2015 at 04:27:03PM +0100, Marc Zyngier wrote:
>> On 04/08/15 14:04, Christoffer Dall wrote:
>>> On Fri, Jul 24, 2015 at 04:55:04PM +0100, Marc Zyngier wrote:
>>>> In order to be able to feed physical interrupts to a guest, we need
>>>> to be able to establish the virtual-physical mapping between the two
>>>> worlds.
>>>>
>>>> The mappings are kept in a set of RCU lists, indexed by virtual interrupts.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>>>> ---
>>>> arch/arm/kvm/arm.c | 2 +
>>>> include/kvm/arm_vgic.h | 26 +++++++
>>>> virt/kvm/arm/vgic.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>>> 3 files changed, 216 insertions(+), 1 deletion(-)
>>>>
[...]
>>>> +/**
>>>> + * kvm_vgic_unmap_phys_irq - Remove a virtual to physical IRQ mapping
>>>> + * @vcpu: The VCPU pointer
>>>> + * @map: The pointer to a mapping obtained through kvm_vgic_map_phys_irq
>>>> + *
>>>> + * Remove an existing mapping between virtual and physical interrupts.
>>>> + */
>>>> +int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, struct irq_phys_map *map)
>>>> +{
>>>> + struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>>>> + struct irq_phys_map_entry *entry;
>>>> + struct list_head *root;
>>>> +
>>>> + if (!map)
>>>> + return -EINVAL;
>>>> +
>>>> + root = vgic_get_irq_phys_map_list(vcpu, map->virt_irq);
>>>> +
>>>> + spin_lock(&dist->irq_phys_map_lock);
>>>> +
>>>> + list_for_each_entry(entry, root, entry) {
>>>> + if (&entry->map == map && !map->deleted) {
>>>> + map->deleted = true;
>>>
>>> why do you need the 'deleted' flag? You now search the list only after
>>> holding the lock, so the race I pointed out before doesn't exist
>>> anymore. Is there an additional issue that the deleted flag is taking
>>> care of?
>>
>> This could still race with a destroy occuring before we take the lock,
>> and before we get get RCU to do the cleanup (the list would still be
>> populated).
>>
>
> That's not how I understand list_del_rcu; I think it deletes the entry
> immediately with the right memory barriers. It is only the free
> operation that happens on rcu sync and can be/is deferred.
Indeed. I stupidly though the two were deferred, but reading the code
definitely confirmed your point. I'll get rid of the delete stuff.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
