* [PATCH V2 2/6] Drivers: hv: vmbus: Introduce a function to remove a rescinded offer
2015-02-28 19:18 ` [PATCH V2 1/6] Drivers: hv: vmbus: Properly handle child device remove K. Y. Srinivasan
@ 2015-02-28 19:18 ` K. Y. Srinivasan
2015-02-28 19:18 ` [PATCH V2 3/6] Drivers: hv: vmbus: Handle both rescind and offer messages in the same context K. Y. Srinivasan
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: K. Y. Srinivasan @ 2015-02-28 19:18 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan
In response to a rescind message, we need to remove the channel and the
corresponding device. Cleanup this code path by factoring out the code
to remove a channel.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel.c | 9 ++++++++
drivers/hv/channel_mgmt.c | 49 +++++++++++++++++++++++++++-----------------
drivers/hv/vmbus_drv.c | 11 +++++++++-
include/linux/hyperv.h | 1 +
4 files changed, 50 insertions(+), 20 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index bf0cf8f..9b79aca 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -501,6 +501,15 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
put_cpu();
}
+ /*
+ * If the channel has been rescinded; process device removal.
+ */
+ if (channel->rescind) {
+ hv_process_channel_removal(channel,
+ channel->offermsg.child_relid);
+ return 0;
+ }
+
/* Send a closing message */
msg = &channel->close_msg.msg;
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 0ba6b5c..b933891 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -207,33 +207,21 @@ static void percpu_channel_deq(void *arg)
list_del(&channel->percpu_list);
}
-/*
- * vmbus_process_rescind_offer -
- * Rescind the offer by initiating a device removal
- */
-static void vmbus_process_rescind_offer(struct work_struct *work)
+
+void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
{
- struct vmbus_channel *channel = container_of(work,
- struct vmbus_channel,
- work);
+ struct vmbus_channel_relid_released msg;
unsigned long flags;
struct vmbus_channel *primary_channel;
- struct vmbus_channel_relid_released msg;
- struct device *dev;
-
- if (channel->device_obj) {
- dev = get_device(&channel->device_obj->device);
- if (dev) {
- vmbus_device_unregister(channel->device_obj);
- put_device(dev);
- }
- }
memset(&msg, 0, sizeof(struct vmbus_channel_relid_released));
- msg.child_relid = channel->offermsg.child_relid;
+ msg.child_relid = relid;
msg.header.msgtype = CHANNELMSG_RELID_RELEASED;
vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released));
+ if (channel == NULL)
+ return;
+
if (channel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(channel->target_cpu,
@@ -256,6 +244,29 @@ static void vmbus_process_rescind_offer(struct work_struct *work)
free_channel(channel);
}
+/*
+ * vmbus_process_rescind_offer -
+ * Rescind the offer by initiating a device removal
+ */
+static void vmbus_process_rescind_offer(struct work_struct *work)
+{
+ struct vmbus_channel *channel = container_of(work,
+ struct vmbus_channel,
+ work);
+ struct device *dev;
+
+ if (channel->device_obj) {
+ dev = get_device(&channel->device_obj->device);
+ if (dev) {
+ vmbus_device_unregister(channel->device_obj);
+ put_device(dev);
+ }
+ } else {
+ hv_process_channel_removal(channel,
+ channel->offermsg.child_relid);
+ }
+}
+
void vmbus_free_channels(void)
{
struct vmbus_channel *channel;
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 04bdc0f..f84ce5e 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -510,14 +510,23 @@ static int vmbus_remove(struct device *child_device)
{
struct hv_driver *drv;
struct hv_device *dev = device_to_hv_device(child_device);
+ u32 relid = dev->channel->offermsg.child_relid;
if (child_device->driver) {
drv = drv_to_hv_drv(child_device->driver);
if (drv->remove)
drv->remove(dev);
- else
+ else {
+ hv_process_channel_removal(dev->channel, relid);
pr_err("remove not set for driver %s\n",
dev_name(child_device));
+ }
+ } else {
+ /*
+ * We don't have a driver for this device; deal with the
+ * rescind message by removing the channel.
+ */
+ hv_process_channel_removal(dev->channel, relid);
}
return 0;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 7d976ac..dd92a85 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1226,6 +1226,7 @@ void hv_kvp_onchannelcallback(void *);
int hv_vss_init(struct hv_util_service *);
void hv_vss_deinit(void);
void hv_vss_onchannelcallback(void *);
+void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid);
extern struct resource hyperv_mmio;
--
1.7.4.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH V2 3/6] Drivers: hv: vmbus: Handle both rescind and offer messages in the same context
2015-02-28 19:18 ` [PATCH V2 1/6] Drivers: hv: vmbus: Properly handle child device remove K. Y. Srinivasan
2015-02-28 19:18 ` [PATCH V2 2/6] Drivers: hv: vmbus: Introduce a function to remove a rescinded offer K. Y. Srinivasan
@ 2015-02-28 19:18 ` K. Y. Srinivasan
2015-02-28 19:18 ` [PATCH V2 4/6] Drivers: hv: vmbus: Remove the channel from the channel list(s) on failure K. Y. Srinivasan
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: K. Y. Srinivasan @ 2015-02-28 19:18 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan
Execute both ressind and offer messages in the same work context. This serializes these
operations and naturally addresses the various corner cases.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
- Execute blocking operations in the per-channel work elements [Dexuan Cui <decui@microsoft.com>]
drivers/hv/channel_mgmt.c | 115 ++++++++++++++++++++++++---------------------
1 files changed, 62 insertions(+), 53 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index b933891..f8528e1 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -134,6 +134,34 @@ fw_error:
EXPORT_SYMBOL_GPL(vmbus_prep_negotiate_resp);
+static void vmbus_process_device_unregister(struct work_struct *work)
+{
+ struct device *dev;
+ struct vmbus_channel *channel = container_of(work,
+ struct vmbus_channel,
+ work);
+
+ dev = get_device(&channel->device_obj->device);
+ if (dev) {
+ vmbus_device_unregister(channel->device_obj);
+ put_device(dev);
+ }
+}
+
+static void vmbus_sc_creation_cb(struct work_struct *work)
+{
+ struct vmbus_channel *newchannel = container_of(work,
+ struct vmbus_channel,
+ work);
+ struct vmbus_channel *primary_channel = newchannel->primary_channel;
+
+ /*
+ * On entry sc_creation_callback has been already verified to
+ * be non-NULL.
+ */
+ primary_channel->sc_creation_callback(newchannel);
+}
+
/*
* alloc_channel - Allocate and initialize a vmbus channel object
*/
@@ -244,29 +272,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
free_channel(channel);
}
-/*
- * vmbus_process_rescind_offer -
- * Rescind the offer by initiating a device removal
- */
-static void vmbus_process_rescind_offer(struct work_struct *work)
-{
- struct vmbus_channel *channel = container_of(work,
- struct vmbus_channel,
- work);
- struct device *dev;
-
- if (channel->device_obj) {
- dev = get_device(&channel->device_obj->device);
- if (dev) {
- vmbus_device_unregister(channel->device_obj);
- put_device(dev);
- }
- } else {
- hv_process_channel_removal(channel,
- channel->offermsg.child_relid);
- }
-}
-
void vmbus_free_channels(void)
{
struct vmbus_channel *channel;
@@ -281,11 +286,8 @@ void vmbus_free_channels(void)
* vmbus_process_offer - Process the offer by creating a channel/device
* associated with this offer
*/
-static void vmbus_process_offer(struct work_struct *work)
+static void vmbus_process_offer(struct vmbus_channel *newchannel)
{
- struct vmbus_channel *newchannel = container_of(work,
- struct vmbus_channel,
- work);
struct vmbus_channel *channel;
bool fnew = true;
bool enq = false;
@@ -349,9 +351,19 @@ static void vmbus_process_offer(struct work_struct *work)
newchannel->state = CHANNEL_OPEN_STATE;
if (channel->sc_creation_callback != NULL)
- channel->sc_creation_callback(newchannel);
-
- goto done_init_rescind;
+ /*
+ * We need to invoke the sub-channel creation
+ * callback; invoke this in a seperate work
+ * context since we are currently running on
+ * the global work context in which we handle
+ * messages from the host.
+ */
+ INIT_WORK(&newchannel->work,
+ vmbus_sc_creation_cb);
+ queue_work(newchannel->controlwq,
+ &newchannel->work);
+
+ return;
}
goto err_free_chan;
@@ -392,15 +404,9 @@ static void vmbus_process_offer(struct work_struct *work)
kfree(newchannel->device_obj);
goto err_free_chan;
}
-done_init_rescind:
- spin_lock_irqsave(&newchannel->lock, flags);
- /* The next possible work is rescind handling */
- INIT_WORK(&newchannel->work, vmbus_process_rescind_offer);
- /* Check if rescind offer was already received */
- if (newchannel->rescind)
- queue_work(newchannel->controlwq, &newchannel->work);
- spin_unlock_irqrestore(&newchannel->lock, flags);
+
return;
+
err_free_chan:
free_channel(newchannel);
}
@@ -526,8 +532,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
newchannel->monitor_grp = (u8)offer->monitorid / 32;
newchannel->monitor_bit = (u8)offer->monitorid % 32;
- INIT_WORK(&newchannel->work, vmbus_process_offer);
- queue_work(newchannel->controlwq, &newchannel->work);
+ vmbus_process_offer(newchannel);
}
/*
@@ -544,24 +549,28 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
rescind = (struct vmbus_channel_rescind_offer *)hdr;
channel = relid2channel(rescind->child_relid);
- if (channel == NULL)
- /* Just return here, no channel found */
+ if (channel == NULL) {
+ hv_process_channel_removal(NULL, rescind->child_relid);
return;
+ }
spin_lock_irqsave(&channel->lock, flags);
channel->rescind = true;
- /*
- * channel->work.func != vmbus_process_rescind_offer means we are still
- * processing offer request and the rescind offer processing should be
- * postponed. It will be done at the very end of vmbus_process_offer()
- * as rescind flag is being checked there.
- */
- if (channel->work.func == vmbus_process_rescind_offer)
- /* work is initialized for vmbus_process_rescind_offer() from
- * vmbus_process_offer() where the channel got created */
- queue_work(channel->controlwq, &channel->work);
-
spin_unlock_irqrestore(&channel->lock, flags);
+
+ if (channel->device_obj) {
+ /*
+ * We will have to unregister this device from the
+ * driver core. Do this in the per-channel work context.
+ * Note that we are currently executing on the global
+ * workq for handling messages from the host.
+ */
+ INIT_WORK(&channel->work, vmbus_process_device_unregister);
+ queue_work(channel->controlwq, &channel->work);
+ } else {
+ hv_process_channel_removal(channel,
+ channel->offermsg.child_relid);
+ }
}
/*
--
1.7.4.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH V2 4/6] Drivers: hv: vmbus: Remove the channel from the channel list(s) on failure
2015-02-28 19:18 ` [PATCH V2 1/6] Drivers: hv: vmbus: Properly handle child device remove K. Y. Srinivasan
2015-02-28 19:18 ` [PATCH V2 2/6] Drivers: hv: vmbus: Introduce a function to remove a rescinded offer K. Y. Srinivasan
2015-02-28 19:18 ` [PATCH V2 3/6] Drivers: hv: vmbus: Handle both rescind and offer messages in the same context K. Y. Srinivasan
@ 2015-02-28 19:18 ` K. Y. Srinivasan
2015-02-28 19:18 ` [PATCH V2 5/6] Drivers: hv: util: On device remove, close the channel after de-initializing the service K. Y. Srinivasan
2015-02-28 19:18 ` [PATCH V2 6/6] Drivers: hv: vmbus: Get rid of some unnecessary messages K. Y. Srinivasan
4 siblings, 0 replies; 7+ messages in thread
From: K. Y. Srinivasan @ 2015-02-28 19:18 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan
Properly rollback state in vmbus_pocess_offer() in the failure paths.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel_mgmt.c | 21 ++++++++++++++++-----
1 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 6c1f792..cc6996a 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -384,7 +384,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
&newchannel->offermsg.offer.if_instance,
newchannel);
if (!newchannel->device_obj)
- goto err_free_chan;
+ goto err_deq_chan;
/*
* Add the new device to the bus. This will kick off device-driver
@@ -396,15 +396,26 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
pr_err("unable to add child device object (relid %d)\n",
newchannel->offermsg.child_relid);
- spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
- list_del(&newchannel->listentry);
- spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
kfree(newchannel->device_obj);
- goto err_free_chan;
+ goto err_deq_chan;
}
return;
+err_deq_chan:
+ spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
+ list_del(&newchannel->listentry);
+ spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
+
+ if (newchannel->target_cpu != get_cpu()) {
+ put_cpu();
+ smp_call_function_single(newchannel->target_cpu,
+ percpu_channel_deq, newchannel, true);
+ } else {
+ percpu_channel_deq(newchannel);
+ put_cpu();
+ }
+
err_free_chan:
free_channel(newchannel);
}
--
1.7.4.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH V2 5/6] Drivers: hv: util: On device remove, close the channel after de-initializing the service
2015-02-28 19:18 ` [PATCH V2 1/6] Drivers: hv: vmbus: Properly handle child device remove K. Y. Srinivasan
` (2 preceding siblings ...)
2015-02-28 19:18 ` [PATCH V2 4/6] Drivers: hv: vmbus: Remove the channel from the channel list(s) on failure K. Y. Srinivasan
@ 2015-02-28 19:18 ` K. Y. Srinivasan
2015-02-28 19:18 ` [PATCH V2 6/6] Drivers: hv: vmbus: Get rid of some unnecessary messages K. Y. Srinivasan
4 siblings, 0 replies; 7+ messages in thread
From: K. Y. Srinivasan @ 2015-02-28 19:18 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan
When the offer is rescinded, vmbus_close() can free up the channel;
deinitialize the service before closing the channel.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/hv_util.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index c5be773..7994ec2 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -380,9 +380,9 @@ static int util_remove(struct hv_device *dev)
{
struct hv_util_service *srv = hv_get_drvdata(dev);
- vmbus_close(dev->channel);
if (srv->util_deinit)
srv->util_deinit();
+ vmbus_close(dev->channel);
kfree(srv->recv_buffer);
return 0;
--
1.7.4.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH V2 6/6] Drivers: hv: vmbus: Get rid of some unnecessary messages
2015-02-28 19:18 ` [PATCH V2 1/6] Drivers: hv: vmbus: Properly handle child device remove K. Y. Srinivasan
` (3 preceding siblings ...)
2015-02-28 19:18 ` [PATCH V2 5/6] Drivers: hv: util: On device remove, close the channel after de-initializing the service K. Y. Srinivasan
@ 2015-02-28 19:18 ` K. Y. Srinivasan
4 siblings, 0 replies; 7+ messages in thread
From: K. Y. Srinivasan @ 2015-02-28 19:18 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan
Currently we log messages when either we are not able to map an ID to a
channel or when the channel does not have a callback associated
(in the channel interrupt handling path). These messages don't add
any value, get rid of them.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/connection.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index af2388f..583d7d4 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -318,10 +318,8 @@ static void process_chn_event(u32 relid)
*/
channel = pcpu_relid2channel(relid);
- if (!channel) {
- pr_err("channel not found for relid - %u\n", relid);
+ if (!channel)
return;
- }
/*
* A channel once created is persistent even when there
@@ -356,10 +354,7 @@ static void process_chn_event(u32 relid)
else
bytes_to_read = 0;
} while (read_state && (bytes_to_read != 0));
- } else {
- pr_err("no channel callback for relid - %u\n", relid);
}
-
}
/*
--
1.7.4.1
^ permalink raw reply [flat|nested] 7+ messages in thread