[PATCH 2/4] hv: fcopy: use wrapper to propate state

From: Olaf Hering
Date: Mon Sep 07 2015 - 10:33:33 EST


The .state is used by several threads of execution.
Propagate the state to make changes visible. Also propagate context
change in hv_fcopy_onchannelcallback.
Without this change fcopy may hang at random points.

Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
---
drivers/hv/hv_fcopy.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index db4b887..47d9c34 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -46,7 +46,7 @@
*/

static struct {
- int state; /* hvutil_device_state */
+ enum hvutil_device_state state;
int recv_len; /* number of bytes received. */
struct hv_fcopy_hdr *fcopy_msg; /* current message */
struct vmbus_channel *recv_channel; /* chn we got the request */
@@ -67,6 +67,9 @@ static struct hvutil_transport *hvt;
*/
static int dm_reg_value;

+#define fcopy_get_state() hvutil_device_get_state(&fcopy_transaction.state)
+#define fcopy_set_state(s) hvutil_device_set_state(&fcopy_transaction.state, s)
+
static void fcopy_timeout_func(struct work_struct *dummy)
{
/*
@@ -76,8 +79,8 @@ static void fcopy_timeout_func(struct work_struct *dummy)
fcopy_respond_to_host(HV_E_FAIL);

/* Transaction is finished, reset the state. */
- if (fcopy_transaction.state > HVUTIL_READY)
- fcopy_transaction.state = HVUTIL_READY;
+ if (fcopy_get_state() > HVUTIL_READY)
+ fcopy_set_state(HVUTIL_READY);

hv_poll_channel(fcopy_transaction.fcopy_context,
hv_fcopy_onchannelcallback);
@@ -108,7 +111,7 @@ static int fcopy_handle_handshake(u32 version)
return -EINVAL;
}
pr_debug("FCP: userspace daemon ver. %d registered\n", version);
- fcopy_transaction.state = HVUTIL_READY;
+ fcopy_set_state(HVUTIL_READY);
hv_poll_channel(fcopy_transaction.fcopy_context,
hv_fcopy_onchannelcallback);
return 0;
@@ -162,13 +165,13 @@ static void fcopy_send_data(struct work_struct *dummy)
break;
}

- fcopy_transaction.state = HVUTIL_USERSPACE_REQ;
+ fcopy_set_state(HVUTIL_USERSPACE_REQ);
rc = hvutil_transport_send(hvt, out_src, out_len);
if (rc) {
pr_debug("FCP: failed to communicate to the daemon: %d\n", rc);
if (cancel_delayed_work_sync(&fcopy_timeout_work)) {
fcopy_respond_to_host(HV_E_FAIL);
- fcopy_transaction.state = HVUTIL_READY;
+ fcopy_set_state(HVUTIL_READY);
}
}
kfree(smsg_out);
@@ -227,12 +230,13 @@ void hv_fcopy_onchannelcallback(void *context)
int util_fw_version;
int fcopy_srv_version;

- if (fcopy_transaction.state > HVUTIL_READY) {
+ if (fcopy_get_state() > HVUTIL_READY) {
/*
* We will defer processing this callback once
* the current transaction is complete.
*/
fcopy_transaction.fcopy_context = context;
+ wmb();
return;
}
fcopy_transaction.fcopy_context = NULL;
@@ -264,12 +268,12 @@ void hv_fcopy_onchannelcallback(void *context)
fcopy_transaction.recv_req_id = requestid;
fcopy_transaction.fcopy_msg = fcopy_msg;

- if (fcopy_transaction.state < HVUTIL_READY) {
+ if (fcopy_get_state() < HVUTIL_READY) {
/* Userspace is not registered yet */
fcopy_respond_to_host(HV_E_FAIL);
return;
}
- fcopy_transaction.state = HVUTIL_HOSTMSG_RECEIVED;
+ fcopy_set_state(HVUTIL_HOSTMSG_RECEIVED);

/*
* Send the information to the user-level daemon.
@@ -291,10 +295,10 @@ static int fcopy_on_msg(void *msg, int len)
if (len != sizeof(int))
return -EINVAL;

- if (fcopy_transaction.state == HVUTIL_DEVICE_INIT)
+ if (fcopy_get_state() == HVUTIL_DEVICE_INIT)
return fcopy_handle_handshake(*val);

- if (fcopy_transaction.state != HVUTIL_USERSPACE_REQ)
+ if (fcopy_get_state() != HVUTIL_USERSPACE_REQ)
return -EINVAL;

/*
@@ -302,9 +306,9 @@ static int fcopy_on_msg(void *msg, int len)
* to the host. But first, cancel the timeout.
*/
if (cancel_delayed_work_sync(&fcopy_timeout_work)) {
- fcopy_transaction.state = HVUTIL_USERSPACE_RECV;
+ fcopy_set_state(HVUTIL_USERSPACE_RECV);
fcopy_respond_to_host(*val);
- fcopy_transaction.state = HVUTIL_READY;
+ fcopy_set_state(HVUTIL_READY);
hv_poll_channel(fcopy_transaction.fcopy_context,
hv_fcopy_onchannelcallback);
}
@@ -317,7 +321,7 @@ static void fcopy_on_reset(void)
/*
* The daemon has exited; reset the state.
*/
- fcopy_transaction.state = HVUTIL_DEVICE_INIT;
+ fcopy_set_state(HVUTIL_DEVICE_INIT);

if (cancel_delayed_work_sync(&fcopy_timeout_work))
fcopy_respond_to_host(HV_E_FAIL);
@@ -333,7 +337,7 @@ int hv_fcopy_init(struct hv_util_service *srv)
* Defer processing channel callbacks until the daemon
* has registered.
*/
- fcopy_transaction.state = HVUTIL_DEVICE_INIT;
+ fcopy_set_state(HVUTIL_DEVICE_INIT);

hvt = hvutil_transport_init(fcopy_devname, 0, 0,
fcopy_on_msg, fcopy_on_reset);
@@ -345,7 +349,7 @@ int hv_fcopy_init(struct hv_util_service *srv)

void hv_fcopy_deinit(void)
{
- fcopy_transaction.state = HVUTIL_DEVICE_DYING;
+ fcopy_set_state(HVUTIL_DEVICE_DYING);
cancel_delayed_work_sync(&fcopy_timeout_work);
hvutil_transport_destroy(hvt);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/