| Message ID | 1362043352-56889-4-git-send-email-cornelia.huck@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 02/28/2013 04:22 AM, Cornelia Huck wrote: > Currently, eventfd introduces module_init/module_exit functions > to initialize/cleanup the irqfd workqueue. This only works, however, > if no other module_init/module_exit functions are built into the > same module. > > Let's just move the initialization and cleanup to kvm_init and kvm_exit. > This way, it is also clearer where kvm startup may fail. > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> I'm seeing this during boot: [ 6.763302] ------------[ cut here ]------------ [ 6.763763] WARNING: at kernel/workqueue.c:4204 destroy_workqueue+0x1df/0x3d0() [ 6.764507] Modules linked in: [ 6.764792] Pid: 1, comm: swapper/0 Tainted: G W 3.9.0-rc5-next-20130402-sasha-00015-g3522ec5 #324 [ 6.765654] Call Trace: [ 6.765875] [<ffffffff811074fb>] warn_slowpath_common+0x8b/0xc0 [ 6.766436] [<ffffffff81107545>] warn_slowpath_null+0x15/0x20 [ 6.766947] [<ffffffff8112ca7f>] destroy_workqueue+0x1df/0x3d0 [ 6.768631] [<ffffffff8100d880>] kvm_irqfd_exit+0x10/0x20 [ 6.770000] [<ffffffff81004dbb>] kvm_init+0x2ab/0x310 [ 6.770607] [<ffffffff86183dc0>] ? cpu_has_kvm_support+0x4d/0x4d [ 6.771241] [<ffffffff86183fb4>] vmx_init+0x1f4/0x437 [ 6.771709] [<ffffffff86183dc0>] ? cpu_has_kvm_support+0x4d/0x4d [ 6.772266] [<ffffffff810020f2>] do_one_initcall+0xb2/0x1b0 [ 6.772995] [<ffffffff86180021>] kernel_init_freeable+0x15d/0x1ef [ 6.773857] [<ffffffff8617f801>] ? loglevel+0x31/0x31 [ 6.774609] [<ffffffff83d51230>] ? rest_init+0x140/0x140 [ 6.775551] [<ffffffff83d51239>] kernel_init+0x9/0xf0 [ 6.776162] [<ffffffff83dbf37c>] ret_from_fork+0x7c/0xb0 [ 6.776662] [<ffffffff83d51230>] ? rest_init+0x140/0x140 [ 6.777241] ---[ end trace 10bba684ced4346a ]--- And I think it has something to do with this patch. Thanks, Sasha
On Tue, 02 Apr 2013 20:40:53 -0400 Sasha Levin <sasha.levin@oracle.com> wrote: > On 02/28/2013 04:22 AM, Cornelia Huck wrote: > > Currently, eventfd introduces module_init/module_exit functions > > to initialize/cleanup the irqfd workqueue. This only works, however, > > if no other module_init/module_exit functions are built into the > > same module. > > > > Let's just move the initialization and cleanup to kvm_init and kvm_exit. > > This way, it is also clearer where kvm startup may fail. > > > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > > I'm seeing this during boot: > > [ 6.763302] ------------[ cut here ]------------ > [ 6.763763] WARNING: at kernel/workqueue.c:4204 destroy_workqueue+0x1df/0x3d0() > [ 6.764507] Modules linked in: > [ 6.764792] Pid: 1, comm: swapper/0 Tainted: G W 3.9.0-rc5-next-20130402-sasha-00015-g3522ec5 #324 > [ 6.765654] Call Trace: > [ 6.765875] [<ffffffff811074fb>] warn_slowpath_common+0x8b/0xc0 > [ 6.766436] [<ffffffff81107545>] warn_slowpath_null+0x15/0x20 > [ 6.766947] [<ffffffff8112ca7f>] destroy_workqueue+0x1df/0x3d0 > [ 6.768631] [<ffffffff8100d880>] kvm_irqfd_exit+0x10/0x20 > [ 6.770000] [<ffffffff81004dbb>] kvm_init+0x2ab/0x310 > [ 6.770607] [<ffffffff86183dc0>] ? cpu_has_kvm_support+0x4d/0x4d > [ 6.771241] [<ffffffff86183fb4>] vmx_init+0x1f4/0x437 > [ 6.771709] [<ffffffff86183dc0>] ? cpu_has_kvm_support+0x4d/0x4d > [ 6.772266] [<ffffffff810020f2>] do_one_initcall+0xb2/0x1b0 > [ 6.772995] [<ffffffff86180021>] kernel_init_freeable+0x15d/0x1ef > [ 6.773857] [<ffffffff8617f801>] ? loglevel+0x31/0x31 > [ 6.774609] [<ffffffff83d51230>] ? rest_init+0x140/0x140 > [ 6.775551] [<ffffffff83d51239>] kernel_init+0x9/0xf0 > [ 6.776162] [<ffffffff83dbf37c>] ret_from_fork+0x7c/0xb0 > [ 6.776662] [<ffffffff83d51230>] ? rest_init+0x140/0x140 > [ 6.777241] ---[ end trace 10bba684ced4346a ]--- > > And I think it has something to do with this patch. What this patch does is move initialization of the workqueue - the code called should be indentical. The workqueue created/destroyed should not have been touched (no irqfds yet), but the new workqueue code seems to encounter something unexpected. I'd expect destroy_workqueue() to be callable after a successful create_singlethread_workqueue(). Tejun? > > > Thanks, > Sasha >
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 722cae7..3b768ef 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -423,6 +423,19 @@ void kvm_vcpu_uninit(struct kvm_vcpu *vcpu); int __must_check vcpu_load(struct kvm_vcpu *vcpu); void vcpu_put(struct kvm_vcpu *vcpu); +#ifdef __KVM_HAVE_IOAPIC +int kvm_irqfd_init(void); +void kvm_irqfd_exit(void); +#else +static inline int kvm_irqfd_init(void) +{ + return 0; +} + +static inline void kvm_irqfd_exit(void) +{ +} +#endif int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, struct module *module); void kvm_exit(void); diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c index b6eea5c..f0ced1a 100644 --- a/virt/kvm/eventfd.c +++ b/virt/kvm/eventfd.c @@ -544,7 +544,7 @@ void kvm_irq_routing_update(struct kvm *kvm, * aggregated from all vm* instances. We need our own isolated single-thread * queue to prevent deadlock against flushing the normal work-queue. */ -static int __init irqfd_module_init(void) +int kvm_irqfd_init(void) { irqfd_cleanup_wq = create_singlethread_workqueue("kvm-irqfd-cleanup"); if (!irqfd_cleanup_wq) @@ -553,13 +553,10 @@ static int __init irqfd_module_init(void) return 0; } -static void __exit irqfd_module_exit(void) +void kvm_irqfd_exit(void) { destroy_workqueue(irqfd_cleanup_wq); } - -module_init(irqfd_module_init); -module_exit(irqfd_module_exit); #endif /* diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index adc68fe..7c188a3 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2920,6 +2920,9 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, int r; int cpu; + r = kvm_irqfd_init(); + if (r) + goto out_irqfd; r = kvm_arch_init(opaque); if (r) goto out_fail; @@ -3000,6 +3003,8 @@ out_free_0a: out_free_0: kvm_arch_exit(); out_fail: + kvm_irqfd_exit(); +out_irqfd: return r; } EXPORT_SYMBOL_GPL(kvm_init); @@ -3016,6 +3021,7 @@ void kvm_exit(void) on_each_cpu(hardware_disable_nolock, NULL, 1); kvm_arch_hardware_unsetup(); kvm_arch_exit(); + kvm_irqfd_exit(); free_cpumask_var(cpus_hardware_enabled); } EXPORT_SYMBOL_GPL(kvm_exit);
Currently, eventfd introduces module_init/module_exit functions to initialize/cleanup the irqfd workqueue. This only works, however, if no other module_init/module_exit functions are built into the same module. Let's just move the initialization and cleanup to kvm_init and kvm_exit. This way, it is also clearer where kvm startup may fail. Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> --- include/linux/kvm_host.h | 13 +++++++++++++ virt/kvm/eventfd.c | 7 ++----- virt/kvm/kvm_main.c | 6 ++++++ 3 files changed, 21 insertions(+), 5 deletions(-)