qemu-devel
[Top][All Lists]
Advanced

[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
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]