* [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device()
@ 2009-12-07 8:58 Sheng Yang
2009-12-14 7:14 ` Sheng Yang
2009-12-14 19:12 ` Marcelo Tosatti
0 siblings, 2 replies; 6+ messages in thread
From: Sheng Yang @ 2009-12-07 8:58 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Sheng Yang
One possible order is:
KVM_CREATE_IRQCHIP ioctl(took kvm->lock) -> kvm_iobus_register_dev() ->
down_write(kvm->slots_lock).
The other one is in kvm_vm_ioctl_assign_device(), which take kvm->slots_lock
first, then kvm->lock.
Observe it due to kernel locking debug warnings.
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/assigned-dev.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
index fd9c097..f73de63 100644
--- a/virt/kvm/assigned-dev.c
+++ b/virt/kvm/assigned-dev.c
@@ -508,8 +508,8 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
struct kvm_assigned_dev_kernel *match;
struct pci_dev *dev;
- down_read(&kvm->slots_lock);
mutex_lock(&kvm->lock);
+ down_read(&kvm->slots_lock);
match = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
assigned_dev->assigned_dev_id);
@@ -573,8 +573,8 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
}
out:
- mutex_unlock(&kvm->lock);
up_read(&kvm->slots_lock);
+ mutex_unlock(&kvm->lock);
return r;
out_list_del:
list_del(&match->list);
@@ -585,8 +585,8 @@ out_put:
pci_dev_put(dev);
out_free:
kfree(match);
- mutex_unlock(&kvm->lock);
up_read(&kvm->slots_lock);
+ mutex_unlock(&kvm->lock);
return r;
}
--
1.5.4.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device()
2009-12-07 8:58 [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device() Sheng Yang
@ 2009-12-14 7:14 ` Sheng Yang
2009-12-14 9:36 ` Avi Kivity
2009-12-14 19:12 ` Marcelo Tosatti
1 sibling, 1 reply; 6+ messages in thread
From: Sheng Yang @ 2009-12-14 7:14 UTC (permalink / raw)
To: kvm; +Cc: Avi Kivity, Marcelo Tosatti
On Monday 07 December 2009 16:58:04 Sheng Yang wrote:
> One possible order is:
>
> KVM_CREATE_IRQCHIP ioctl(took kvm->lock) -> kvm_iobus_register_dev() ->
> down_write(kvm->slots_lock).
>
> The other one is in kvm_vm_ioctl_assign_device(), which take
> kvm->slots_lock first, then kvm->lock.
>
> Observe it due to kernel locking debug warnings.
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Avi? Marcelo?
--
regards
Yang, Sheng
> ---
> virt/kvm/assigned-dev.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> index fd9c097..f73de63 100644
> --- a/virt/kvm/assigned-dev.c
> +++ b/virt/kvm/assigned-dev.c
> @@ -508,8 +508,8 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> struct kvm_assigned_dev_kernel *match;
> struct pci_dev *dev;
>
> - down_read(&kvm->slots_lock);
> mutex_lock(&kvm->lock);
> + down_read(&kvm->slots_lock);
>
> match = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
> assigned_dev->assigned_dev_id);
> @@ -573,8 +573,8 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> }
>
> out:
> - mutex_unlock(&kvm->lock);
> up_read(&kvm->slots_lock);
> + mutex_unlock(&kvm->lock);
> return r;
> out_list_del:
> list_del(&match->list);
> @@ -585,8 +585,8 @@ out_put:
> pci_dev_put(dev);
> out_free:
> kfree(match);
> - mutex_unlock(&kvm->lock);
> up_read(&kvm->slots_lock);
> + mutex_unlock(&kvm->lock);
> return r;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device()
2009-12-14 7:14 ` Sheng Yang
@ 2009-12-14 9:36 ` Avi Kivity
0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2009-12-14 9:36 UTC (permalink / raw)
To: Sheng Yang; +Cc: kvm, Marcelo Tosatti
On 12/14/2009 09:14 AM, Sheng Yang wrote:
> On Monday 07 December 2009 16:58:04 Sheng Yang wrote:
>
>> One possible order is:
>>
>> KVM_CREATE_IRQCHIP ioctl(took kvm->lock) -> kvm_iobus_register_dev() ->
>> down_write(kvm->slots_lock).
>>
>> The other one is in kvm_vm_ioctl_assign_device(), which take
>> kvm->slots_lock first, then kvm->lock.
>>
>> Observe it due to kernel locking debug warnings.
>>
>> Signed-off-by: Sheng Yang<sheng@linux.intel.com>
>>
> Avi? Marcelo?
>
>
Sorry, hunting a regression so the tree is frozen. Will review soon.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device()
2009-12-07 8:58 [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device() Sheng Yang
2009-12-14 7:14 ` Sheng Yang
@ 2009-12-14 19:12 ` Marcelo Tosatti
2009-12-15 2:28 ` Sheng Yang
1 sibling, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2009-12-14 19:12 UTC (permalink / raw)
To: Sheng Yang; +Cc: Avi Kivity, kvm
On Mon, Dec 07, 2009 at 04:58:04PM +0800, Sheng Yang wrote:
> One possible order is:
>
> KVM_CREATE_IRQCHIP ioctl(took kvm->lock) -> kvm_iobus_register_dev() ->
> down_write(kvm->slots_lock).
>
> The other one is in kvm_vm_ioctl_assign_device(), which take kvm->slots_lock
> first, then kvm->lock.
>
> Observe it due to kernel locking debug warnings.
Sheng,
Looks good.
Can you also update the comment on top of kvm_main.c?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device()
2009-12-14 19:12 ` Marcelo Tosatti
@ 2009-12-15 2:28 ` Sheng Yang
2009-12-15 9:35 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Sheng Yang @ 2009-12-15 2:28 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, kvm, Sheng Yang
One possible order is:
KVM_CREATE_IRQCHIP ioctl(took kvm->lock) -> kvm_iobus_register_dev() ->
down_write(kvm->slots_lock).
The other one is in kvm_vm_ioctl_assign_device(), which take kvm->slots_lock
first, then kvm->lock.
Update the comment of lock order as well.
Observe it due to kernel locking debug warnings.
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
virt/kvm/assigned-dev.c | 6 +++---
virt/kvm/kvm_main.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
index fd9c097..f73de63 100644
--- a/virt/kvm/assigned-dev.c
+++ b/virt/kvm/assigned-dev.c
@@ -508,8 +508,8 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
struct kvm_assigned_dev_kernel *match;
struct pci_dev *dev;
- down_read(&kvm->slots_lock);
mutex_lock(&kvm->lock);
+ down_read(&kvm->slots_lock);
match = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
assigned_dev->assigned_dev_id);
@@ -573,8 +573,8 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
}
out:
- mutex_unlock(&kvm->lock);
up_read(&kvm->slots_lock);
+ mutex_unlock(&kvm->lock);
return r;
out_list_del:
list_del(&match->list);
@@ -585,8 +585,8 @@ out_put:
pci_dev_put(dev);
out_free:
kfree(match);
- mutex_unlock(&kvm->lock);
up_read(&kvm->slots_lock);
+ mutex_unlock(&kvm->lock);
return r;
}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f92ba13..91b84c9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -63,7 +63,7 @@ MODULE_LICENSE("GPL");
/*
* Ordering of locks:
*
- * kvm->slots_lock --> kvm->lock --> kvm->irq_lock
+ * kvm->lock --> kvm->slots_lock --> kvm->irq_lock
*/
DEFINE_SPINLOCK(kvm_lock);
--
1.5.4.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device()
2009-12-15 2:28 ` Sheng Yang
@ 2009-12-15 9:35 ` Avi Kivity
0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2009-12-15 9:35 UTC (permalink / raw)
To: Sheng Yang; +Cc: Marcelo Tosatti, kvm
On 12/15/2009 04:28 AM, Sheng Yang wrote:
> One possible order is:
>
> KVM_CREATE_IRQCHIP ioctl(took kvm->lock) -> kvm_iobus_register_dev() ->
> down_write(kvm->slots_lock).
>
> The other one is in kvm_vm_ioctl_assign_device(), which take kvm->slots_lock
> first, then kvm->lock.
>
> Update the comment of lock order as well.
>
> Observe it due to kernel locking debug warnings.
>
Applied and queued for 2.6.33, thanks.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-12-15 9:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-07 8:58 [PATCH] KVM: Fix possible circular locking in kvm_vm_ioctl_assign_device() Sheng Yang
2009-12-14 7:14 ` Sheng Yang
2009-12-14 9:36 ` Avi Kivity
2009-12-14 19:12 ` Marcelo Tosatti
2009-12-15 2:28 ` Sheng Yang
2009-12-15 9:35 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox