* [PATCH 2/5] Drivers: hv: vmbus: unregister panic notifier on module unload
2015-04-20 7:45 ` [PATCH 1/5] Drivers: hv: vmbus: introduce vmbus_acpi_remove K. Y. Srinivasan
@ 2015-04-20 7:45 ` K. Y. Srinivasan
2015-04-20 7:45 ` [PATCH 3/5] hv: vmbus_free_channels(): remove the redundant free_channel() K. Y. Srinivasan
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: K. Y. Srinivasan @ 2015-04-20 7:45 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang
Cc: K. Y. Srinivasan
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Commit 96c1d0581d00f7abe033350edb021a9d947d8d81 ("Drivers: hv: vmbus: Add
support for VMBus panic notifier handler") introduced
atomic_notifier_chain_register() call on module load. We also need to call
atomic_notifier_chain_unregister() on module unload as otherwise the following
crash is observed when we bring hv_vmbus back:
[ 39.788877] BUG: unable to handle kernel paging request at ffffffffa00078a8
[ 39.788877] IP: [<ffffffff8109d63f>] notifier_call_chain+0x3f/0x80
...
[ 39.788877] Call Trace:
[ 39.788877] [<ffffffff8109de7d>] __atomic_notifier_call_chain+0x5d/0x90
...
[ 39.788877] [<ffffffff8109d788>] ? atomic_notifier_chain_register+0x38/0x70
[ 39.788877] [<ffffffff8109d767>] ? atomic_notifier_chain_register+0x17/0x70
[ 39.788877] [<ffffffffa002814f>] hv_acpi_init+0x14f/0x1000 [hv_vmbus]
[ 39.788877] [<ffffffff81002144>] do_one_initcall+0xd4/0x210
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/vmbus_drv.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 0d8d1d7..7870a90 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1108,6 +1108,10 @@ static void __exit vmbus_exit(void)
hv_synic_clockevents_cleanup();
hv_remove_vmbus_irq();
vmbus_free_channels();
+ if (ms_hyperv.features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+ atomic_notifier_chain_unregister(&panic_notifier_list,
+ &hyperv_panic_block);
+ }
bus_unregister(&hv_bus);
hv_cleanup();
for_each_online_cpu(cpu)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/5] hv: vmbus_free_channels(): remove the redundant free_channel()
2015-04-20 7:45 ` [PATCH 1/5] Drivers: hv: vmbus: introduce vmbus_acpi_remove K. Y. Srinivasan
2015-04-20 7:45 ` [PATCH 2/5] Drivers: hv: vmbus: unregister panic notifier on module unload K. Y. Srinivasan
@ 2015-04-20 7:45 ` K. Y. Srinivasan
2015-04-20 7:45 ` [PATCH 4/5] drivers: hv: vmbus: Get rid of some unused definitions K. Y. Srinivasan
2015-04-20 7:45 ` [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state K. Y. Srinivasan
3 siblings, 0 replies; 11+ messages in thread
From: K. Y. Srinivasan @ 2015-04-20 7:45 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang
Cc: Dexuan Cui, K. Y. Srinivasan
From: Dexuan Cui <decui@microsoft.com>
free_channel() has been invoked in
vmbus_remove() -> hv_process_channel_removal(), or vmbus_remove() ->
... -> vmbus_close_internal() -> hv_process_channel_removal().
We also change to use list_for_each_entry_safe(), because the entry
is removed in hv_process_channel_removal().
This patch fixes a bug in the vmbus unload path.
Thank Dan Carpenter for finding the issue!
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel_mgmt.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 0eeb1b3..865a3af 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -212,11 +212,16 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
void vmbus_free_channels(void)
{
- struct vmbus_channel *channel;
+ struct vmbus_channel *channel, *tmp;
+
+ list_for_each_entry_safe(channel, tmp, &vmbus_connection.chn_list,
+ listentry) {
+ /* if we don't set rescind to true, vmbus_close_internal()
+ * won't invoke hv_process_channel_removal().
+ */
+ channel->rescind = true;
- list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
vmbus_device_unregister(channel->device_obj);
- free_channel(channel);
}
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/5] drivers: hv: vmbus: Get rid of some unused definitions
2015-04-20 7:45 ` [PATCH 1/5] Drivers: hv: vmbus: introduce vmbus_acpi_remove K. Y. Srinivasan
2015-04-20 7:45 ` [PATCH 2/5] Drivers: hv: vmbus: unregister panic notifier on module unload K. Y. Srinivasan
2015-04-20 7:45 ` [PATCH 3/5] hv: vmbus_free_channels(): remove the redundant free_channel() K. Y. Srinivasan
@ 2015-04-20 7:45 ` K. Y. Srinivasan
2015-04-20 7:45 ` [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state K. Y. Srinivasan
3 siblings, 0 replies; 11+ messages in thread
From: K. Y. Srinivasan @ 2015-04-20 7:45 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang
Cc: K. Y. Srinivasan
Get rid of some unused definitions.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
include/linux/hyperv.h | 19 -------------------
1 files changed, 0 insertions(+), 19 deletions(-)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 1744148..e29ccdd 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -389,10 +389,6 @@ enum vmbus_channel_message_type {
CHANNELMSG_INITIATE_CONTACT = 14,
CHANNELMSG_VERSION_RESPONSE = 15,
CHANNELMSG_UNLOAD = 16,
-#ifdef VMBUS_FEATURE_PARENT_OR_PEER_MEMORY_MAPPED_INTO_A_CHILD
- CHANNELMSG_VIEWRANGE_ADD = 17,
- CHANNELMSG_VIEWRANGE_REMOVE = 18,
-#endif
CHANNELMSG_COUNT
};
@@ -549,21 +545,6 @@ struct vmbus_channel_gpadl_torndown {
u32 gpadl;
} __packed;
-#ifdef VMBUS_FEATURE_PARENT_OR_PEER_MEMORY_MAPPED_INTO_A_CHILD
-struct vmbus_channel_view_range_add {
- struct vmbus_channel_message_header header;
- PHYSICAL_ADDRESS viewrange_base;
- u64 viewrange_length;
- u32 child_relid;
-} __packed;
-
-struct vmbus_channel_view_range_remove {
- struct vmbus_channel_message_header header;
- PHYSICAL_ADDRESS viewrange_base;
- u32 child_relid;
-} __packed;
-#endif
-
struct vmbus_channel_relid_released {
struct vmbus_channel_message_header header;
u32 child_relid;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state
2015-04-20 7:45 ` [PATCH 1/5] Drivers: hv: vmbus: introduce vmbus_acpi_remove K. Y. Srinivasan
` (2 preceding siblings ...)
2015-04-20 7:45 ` [PATCH 4/5] drivers: hv: vmbus: Get rid of some unused definitions K. Y. Srinivasan
@ 2015-04-20 7:45 ` K. Y. Srinivasan
2015-04-20 9:39 ` Vitaly Kuznetsov
3 siblings, 1 reply; 11+ messages in thread
From: K. Y. Srinivasan @ 2015-04-20 7:45 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang
Cc: K. Y. Srinivasan
Implement the protocol for tearing down the monitor state established with
the host.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel_mgmt.c | 25 +++++++++++++++++++++++++
drivers/hv/connection.c | 5 +++++
drivers/hv/hyperv_vmbus.h | 2 ++
include/linux/hyperv.h | 1 +
4 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 865a3af..4b9d89a 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -422,6 +422,30 @@ static void init_vp_index(struct vmbus_channel *channel, const uuid_le *type_gui
}
/*
+ * vmbus_unload_response - Handler for the unload response.
+ */
+static void vmbus_unload_response(struct vmbus_channel_message_header *hdr)
+{
+ /*
+ * This is a global event; just wakeup the waiting thread.
+ * Once we successfully unload, we can cleanup the monitor state.
+ */
+ complete(&vmbus_connection.unload_event);
+}
+
+void vmbus_initiate_unload(void)
+{
+ struct vmbus_channel_message_header hdr;
+
+ init_completion(&vmbus_connection.unload_event);
+ memset(&hdr, 0, sizeof(struct vmbus_channel_message_header));
+ hdr.msgtype = CHANNELMSG_UNLOAD;
+ vmbus_post_msg(&hdr, sizeof(struct vmbus_channel_message_header));
+
+ wait_for_completion(&vmbus_connection.unload_event);
+}
+
+/*
* vmbus_onoffer - Handler for channel offers from vmbus in parent partition.
*
*/
@@ -717,6 +741,7 @@ struct vmbus_channel_message_table_entry
{CHANNELMSG_INITIATE_CONTACT, 0, NULL},
{CHANNELMSG_VERSION_RESPONSE, 1, vmbus_onversion_response},
{CHANNELMSG_UNLOAD, 0, NULL},
+ {CHANNELMSG_UNLOAD_RESPONSE, 1, vmbus_unload_response},
};
/*
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index b27220a..acd50e9 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -227,6 +227,11 @@ cleanup:
void vmbus_disconnect(void)
{
+ /*
+ * First send the unload request to the host.
+ */
+ vmbus_initiate_unload();
+
if (vmbus_connection.work_queue) {
drain_workqueue(vmbus_connection.work_queue);
destroy_workqueue(vmbus_connection.work_queue);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 138d663..cddc0c9 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -647,6 +647,7 @@ struct vmbus_connection {
atomic_t next_gpadl_handle;
+ struct completion unload_event;
/*
* Represents channel interrupts. Each bit position represents a
* channel. When a channel sends an interrupt via VMBUS, it finds its
@@ -741,6 +742,7 @@ void hv_vss_onchannelcallback(void *);
int hv_fcopy_init(struct hv_util_service *);
void hv_fcopy_deinit(void);
void hv_fcopy_onchannelcallback(void *);
+void vmbus_initiate_unload(void);
static inline void hv_poll_channel(struct vmbus_channel *channel,
void (*cb)(void *))
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index e29ccdd..ea93486 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -389,6 +389,7 @@ enum vmbus_channel_message_type {
CHANNELMSG_INITIATE_CONTACT = 14,
CHANNELMSG_VERSION_RESPONSE = 15,
CHANNELMSG_UNLOAD = 16,
+ CHANNELMSG_UNLOAD_RESPONSE = 17,
CHANNELMSG_COUNT
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state
2015-04-20 7:45 ` [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state K. Y. Srinivasan
@ 2015-04-20 9:39 ` Vitaly Kuznetsov
2015-04-20 10:28 ` Vitaly Kuznetsov
2015-04-20 14:12 ` KY Srinivasan
0 siblings, 2 replies; 11+ messages in thread
From: Vitaly Kuznetsov @ 2015-04-20 9:39 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang
"K. Y. Srinivasan" <kys@microsoft.com> writes:
> Implement the protocol for tearing down the monitor state established with
> the host.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Unfortunatelly this patch leads to the following crash:
pre-udev:/# modprobe hv_vmbus
modprobe hv_vmbus
[ 14.832334] hv_vmbus: Hyper-V Host Build:9600-6.3-17-0.17039; Vmbus version:3.0
[ 14.834179] hv_vmbus: CPU offlining is not supported by hypervisor
pre-udev:/# modprobe -r hv_vmbus
modprobe -r hv_vmbus
[ 20.640893] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 20.641104] IP: [< (null)>] (null)
[ 20.641104] PGD 3037b067 PUD 2f41c067 PMD 0
[ 20.641104] Oops: 0010 [#1] SMP DEBUG_PAGEALLOC KASAN
[ 20.641104] Modules linked in: hv_vmbus(-)
[ 20.641104] CPU: 4 PID: 296 Comm: modprobe Not tainted 4.0.0-rc5_vmbus_acpi_remove2+ #583
[ 20.641104] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v1.0 11/26/2012
[ 20.641104] task: ffff88002f47e360 ti: ffff88002f618000 task.ti: ffff88002f618000
[ 20.641104] RIP: 0010:[<0000000000000000>] [< (null)>] (null)
[ 20.641104] RSP: 0018:ffff88002f61fe60 EFLAGS: 00010246
[ 20.641104] RAX: 0000000030374000 RBX: ffff880030374000 RCX: 000000000000005c
[ 20.641104] RDX: 0000000030374000 RSI: ffff88002f61fee0 RDI: ffff880030374000
[ 20.641104] RBP: ffff88002f61fe88 R08: 0000000000000000 R09: ffff88002f61fed8
[ 20.641104] R10: ffff880034008240 R11: 0000000000000010 R12: 0000000000000014
[ 20.641104] R13: 0000000000000008 R14: ffff88002f61fed8 R15: 0000000000000000
[ 20.641104] FS: 00007fc1ffdaa700(0000) GS:ffff880034500000(0000) knlGS:0000000000000000
[ 20.641104] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 20.641104] CR2: 0000000000000000 CR3: 000000002f4ec000 CR4: 00000000001407e0
[ 20.641104] Stack:
[ 20.641104] ffffffffa00016c7 0000000000000000 00000000b7f74867 ffffffffa0005ec9
[ 20.641104] 0000000000000001 ffff88002f61fec8 ffffffffa000209c ffff88002f61fec8
[ 20.641104] 0000000000000200 ffffffff8210ccc0 0000000000000001 0000000001c7d460
[ 20.641104] Call Trace:
[ 20.641104] [<ffffffffa00016c7>] ? hv_post_message+0x87/0xb0 [hv_vmbus]
[ 20.641104] [<ffffffffa000209c>] vmbus_post_msg+0x3c/0xc0 [hv_vmbus]
[ 20.641104] [<ffffffffa00045b5>] vmbus_initiate_unload+0x65/0x90 [hv_vmbus]
[ 20.641104] [<ffffffffa0001d7e>] vmbus_disconnect+0xe/0x80 [hv_vmbus]
[ 20.641104] [<ffffffffa0004dd1>] vmbus_exit+0xe9/0x318 [hv_vmbus]
[ 20.641104] [<ffffffff8110704c>] SyS_delete_module+0x1ac/0x230
[ 20.641104] [<ffffffff81aa83f6>] ? int_with_check+0x27/0x69
[ 20.641104] [<ffffffff81aa8209>] system_call_fastpath+0x12/0x17
[ 20.641104] Code: Bad RIP value.
[ 20.641104] RIP [< (null)>] (null)
[ 20.641104] RSP <ffff88002f61fe60>
[ 20.641104] CR2: 0000000000000000
[ 20.708666] ---[ end trace 76d9daf92f0e3af8 ]---
[ 20.709944] Kernel panic - not syncing: Fatal exception
[ 20.710933] Kernel Offset: 0x0 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffff9fffffff)
[ 20.710933] ---[ end Kernel panic - not syncing: Fatal exception
I'll try to investigate.
> ---
> drivers/hv/channel_mgmt.c | 25 +++++++++++++++++++++++++
> drivers/hv/connection.c | 5 +++++
> drivers/hv/hyperv_vmbus.h | 2 ++
> include/linux/hyperv.h | 1 +
> 4 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 865a3af..4b9d89a 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -422,6 +422,30 @@ static void init_vp_index(struct vmbus_channel *channel, const uuid_le *type_gui
> }
>
> /*
> + * vmbus_unload_response - Handler for the unload response.
> + */
> +static void vmbus_unload_response(struct vmbus_channel_message_header *hdr)
> +{
> + /*
> + * This is a global event; just wakeup the waiting thread.
> + * Once we successfully unload, we can cleanup the monitor state.
> + */
> + complete(&vmbus_connection.unload_event);
> +}
> +
> +void vmbus_initiate_unload(void)
> +{
> + struct vmbus_channel_message_header hdr;
> +
> + init_completion(&vmbus_connection.unload_event);
> + memset(&hdr, 0, sizeof(struct vmbus_channel_message_header));
> + hdr.msgtype = CHANNELMSG_UNLOAD;
> + vmbus_post_msg(&hdr, sizeof(struct vmbus_channel_message_header));
> +
> + wait_for_completion(&vmbus_connection.unload_event);
> +}
> +
> +/*
> * vmbus_onoffer - Handler for channel offers from vmbus in parent partition.
> *
> */
> @@ -717,6 +741,7 @@ struct vmbus_channel_message_table_entry
> {CHANNELMSG_INITIATE_CONTACT, 0, NULL},
> {CHANNELMSG_VERSION_RESPONSE, 1, vmbus_onversion_response},
> {CHANNELMSG_UNLOAD, 0, NULL},
> + {CHANNELMSG_UNLOAD_RESPONSE, 1, vmbus_unload_response},
> };
>
> /*
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index b27220a..acd50e9 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -227,6 +227,11 @@ cleanup:
>
> void vmbus_disconnect(void)
> {
> + /*
> + * First send the unload request to the host.
> + */
> + vmbus_initiate_unload();
> +
> if (vmbus_connection.work_queue) {
> drain_workqueue(vmbus_connection.work_queue);
> destroy_workqueue(vmbus_connection.work_queue);
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 138d663..cddc0c9 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -647,6 +647,7 @@ struct vmbus_connection {
>
> atomic_t next_gpadl_handle;
>
> + struct completion unload_event;
> /*
> * Represents channel interrupts. Each bit position represents a
> * channel. When a channel sends an interrupt via VMBUS, it finds its
> @@ -741,6 +742,7 @@ void hv_vss_onchannelcallback(void *);
> int hv_fcopy_init(struct hv_util_service *);
> void hv_fcopy_deinit(void);
> void hv_fcopy_onchannelcallback(void *);
> +void vmbus_initiate_unload(void);
>
> static inline void hv_poll_channel(struct vmbus_channel *channel,
> void (*cb)(void *))
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index e29ccdd..ea93486 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -389,6 +389,7 @@ enum vmbus_channel_message_type {
> CHANNELMSG_INITIATE_CONTACT = 14,
> CHANNELMSG_VERSION_RESPONSE = 15,
> CHANNELMSG_UNLOAD = 16,
> + CHANNELMSG_UNLOAD_RESPONSE = 17,
> CHANNELMSG_COUNT
> };
--
Vitaly
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state
2015-04-20 9:39 ` Vitaly Kuznetsov
@ 2015-04-20 10:28 ` Vitaly Kuznetsov
2015-04-20 14:12 ` KY Srinivasan
1 sibling, 0 replies; 11+ messages in thread
From: Vitaly Kuznetsov @ 2015-04-20 10:28 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang
Vitaly Kuznetsov <vkuznets@redhat.com> writes:
> "K. Y. Srinivasan" <kys@microsoft.com> writes:
>
>> Implement the protocol for tearing down the monitor state established with
>> the host.
>>
>> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>
> Unfortunatelly this patch leads to the following crash:
>
> pre-udev:/# modprobe hv_vmbus
> modprobe hv_vmbus
> [ 14.832334] hv_vmbus: Hyper-V Host Build:9600-6.3-17-0.17039; Vmbus version:3.0
> [ 14.834179] hv_vmbus: CPU offlining is not supported by hypervisor
> pre-udev:/# modprobe -r hv_vmbus
> modprobe -r hv_vmbus
> [ 20.640893] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 20.641104] IP: [< (null)>] (null)
>
> I'll try to investigate.
>
The problem is that we call vmbus_disconnect() too late in
vmbus_exit(). We need to call it:
1) Before hv_cleanup() call as we free hypercall page there (that's what
you see in my crash)
2) Before hv_remove_vmbus_irq() as we're waiting for the hypervisor to
reply.
This simple patch fixes the issue:
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 7870a90..2b56260 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1106,6 +1106,7 @@ static void __exit vmbus_exit(void)
vmbus_connection.conn_state = DISCONNECTED;
hv_synic_clockevents_cleanup();
+ vmbus_disconnect();
hv_remove_vmbus_irq();
vmbus_free_channels();
if (ms_hyperv.features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
@@ -1118,7 +1119,6 @@ static void __exit vmbus_exit(void)
smp_call_function_single(cpu, hv_synic_cleanup, NULL,
1);
acpi_bus_unregister_driver(&vmbus_acpi_driver);
hv_cpu_hotplug_quirk(false);
- vmbus_disconnect();
}
I suggest we incorporate it into your PATCH 5/5.
--
Vitaly
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state
2015-04-20 9:39 ` Vitaly Kuznetsov
2015-04-20 10:28 ` Vitaly Kuznetsov
@ 2015-04-20 14:12 ` KY Srinivasan
2015-04-20 14:17 ` Vitaly Kuznetsov
1 sibling, 1 reply; 11+ messages in thread
From: KY Srinivasan @ 2015-04-20 14:12 UTC (permalink / raw)
To: Vitaly Kuznetsov; +Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang
> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Monday, April 20, 2015 2:40 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for
> tearing down vmbus state
>
> "K. Y. Srinivasan" <kys@microsoft.com> writes:
>
> > Implement the protocol for tearing down the monitor state established
> with
> > the host.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>
> Unfortunatelly this patch leads to the following crash:
Thanks for testing Vitaly. The unload handling is what we want, maybe I need to
Invoke unload a little earlier.
Regards,
K. Y
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state
2015-04-20 14:12 ` KY Srinivasan
@ 2015-04-20 14:17 ` Vitaly Kuznetsov
2015-04-20 14:32 ` KY Srinivasan
0 siblings, 1 reply; 11+ messages in thread
From: Vitaly Kuznetsov @ 2015-04-20 14:17 UTC (permalink / raw)
To: KY Srinivasan; +Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang
KY Srinivasan <kys@microsoft.com> writes:
>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> Sent: Monday, April 20, 2015 2:40 AM
>> To: KY Srinivasan
>> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> jasowang@redhat.com
>> Subject: Re: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for
>> tearing down vmbus state
>>
>> "K. Y. Srinivasan" <kys@microsoft.com> writes:
>>
>> > Implement the protocol for tearing down the monitor state established
>> with
>> > the host.
>> >
>> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>>
>> Unfortunatelly this patch leads to the following crash:
>
> Thanks for testing Vitaly. The unload handling is what we want, maybe I need to
> Invoke unload a little earlier.
I actually sent two letters this morning :-) The other was:
The problem is that we call vmbus_disconnect() too late in
vmbus_exit(). We need to call it:
1) Before hv_cleanup() call as we free hypercall page there (that's what
you see in my crash)
2) Before hv_remove_vmbus_irq() as we're waiting for the hypervisor to
reply.
This simple patch fixes the issue:
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 7870a90..2b56260 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1106,6 +1106,7 @@ static void __exit vmbus_exit(void)
vmbus_connection.conn_state = DISCONNECTED;
hv_synic_clockevents_cleanup();
+ vmbus_disconnect();
hv_remove_vmbus_irq();
vmbus_free_channels();
if (ms_hyperv.features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
@@ -1118,7 +1119,6 @@ static void __exit vmbus_exit(void)
smp_call_function_single(cpu, hv_synic_cleanup, NULL,
1);
acpi_bus_unregister_driver(&vmbus_acpi_driver);
hv_cpu_hotplug_quirk(false);
- vmbus_disconnect();
}
I suggest we incorporate it into your PATCH 5/5.
--
Vitaly
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state
2015-04-20 14:17 ` Vitaly Kuznetsov
@ 2015-04-20 14:32 ` KY Srinivasan
0 siblings, 0 replies; 11+ messages in thread
From: KY Srinivasan @ 2015-04-20 14:32 UTC (permalink / raw)
To: Vitaly Kuznetsov; +Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang
> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Monday, April 20, 2015 7:17 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for
> tearing down vmbus state
>
> KY Srinivasan <kys@microsoft.com> writes:
>
> >> -----Original Message-----
> >> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> >> Sent: Monday, April 20, 2015 2:40 AM
> >> To: KY Srinivasan
> >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> >> jasowang@redhat.com
> >> Subject: Re: [PATCH 5/5] Drivers: hv: vmbus: Implement the protocol for
> >> tearing down vmbus state
> >>
> >> "K. Y. Srinivasan" <kys@microsoft.com> writes:
> >>
> >> > Implement the protocol for tearing down the monitor state established
> >> with
> >> > the host.
> >> >
> >> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> >>
> >> Unfortunatelly this patch leads to the following crash:
> >
> > Thanks for testing Vitaly. The unload handling is what we want, maybe I
> need to
> > Invoke unload a little earlier.
>
> I actually sent two letters this morning :-) The other was:
>
> The problem is that we call vmbus_disconnect() too late in
> vmbus_exit(). We need to call it:
> 1) Before hv_cleanup() call as we free hypercall page there (that's what
> you see in my crash)
> 2) Before hv_remove_vmbus_irq() as we're waiting for the hypervisor to
> reply.
>
> This simple patch fixes the issue:
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 7870a90..2b56260 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1106,6 +1106,7 @@ static void __exit vmbus_exit(void)
>
> vmbus_connection.conn_state = DISCONNECTED;
> hv_synic_clockevents_cleanup();
> + vmbus_disconnect();
> hv_remove_vmbus_irq();
> vmbus_free_channels();
> if (ms_hyperv.features &
> HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
> @@ -1118,7 +1119,6 @@ static void __exit vmbus_exit(void)
> smp_call_function_single(cpu, hv_synic_cleanup, NULL,
> 1);
> acpi_bus_unregister_driver(&vmbus_acpi_driver);
> hv_cpu_hotplug_quirk(false);
> - vmbus_disconnect();
> }
>
> I suggest we incorporate it into your PATCH 5/5.
Thank you; I will resend the patch with your fix.
Regards,
K. Y
>
> --
> Vitaly
^ permalink raw reply [flat|nested] 11+ messages in thread