[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/5] KVM: eventfd: Fix lock order inversion.
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/5] KVM: eventfd: Fix lock order inversion. |
Date: |
Mon, 17 Mar 2014 22:55:49 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 17/03/14 19:11, Cornelia Huck wrote:
> When registering a new irqfd, we call its ->poll method to collect any
> event that might have previously been pending so that we can trigger it.
> This is done under the kvm->irqfds.lock, which means the eventfd's ctx
> lock is taken under it.
>
> However, if we get a POLLHUP in irqfd_wakeup, we will be called with the
> ctx lock held before getting the irqfds.lock to deactivate the irqfd,
> causing lockdep to complain.
>
> Calling the ->poll method does not really need the irqfds.lock, so let's
> just move it after we've given up the irqfds.lock in kvm_irqfd_assign().
>
> Signed-off-by: Cornelia Huck <address@hidden>
Do you still have the lockdep message somewhere?
Looking at the patch and the description this makes sense. Even without
irqfd for s390:
Reviewed-by: Christian Borntraeger <address@hidden>
Paolo, maybe this patch can go in independently from s390?
Christian
> ---
> virt/kvm/eventfd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index abe4d60..29c2a04 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -391,19 +391,19 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd
> *args)
> lockdep_is_held(&kvm->irqfds.lock));
> irqfd_update(kvm, irqfd, irq_rt);
>
> - events = f.file->f_op->poll(f.file, &irqfd->pt);
> -
> list_add_tail(&irqfd->list, &kvm->irqfds.items);
>
> + spin_unlock_irq(&kvm->irqfds.lock);
> +
> /*
> * Check if there was an event already pending on the eventfd
> * before we registered, and trigger it as if we didn't miss it.
> */
> + events = f.file->f_op->poll(f.file, &irqfd->pt);
> +
> if (events & POLLIN)
> schedule_work(&irqfd->inject);
>
> - spin_unlock_irq(&kvm->irqfds.lock);
> -
> /*
> * do not drop the file until the irqfd is fully initialized, otherwise
> * we might race against the POLLHUP
>