public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "K. Y. Srinivasan" <kys@microsoft.com>
To: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com,
	vkuznets@redhat.com, jasowang@redhat.com
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Subject: [PATCH 3/5] hv: fcopy: use wrappers to propagate state
Date: Tue, 15 Sep 2015 17:37:52 -0700	[thread overview]
Message-ID: <1442363874-22508-3-git-send-email-kys@microsoft.com> (raw)
In-Reply-To: <1442363874-22508-1-git-send-email-kys@microsoft.com>

From: Olaf Hering <olaf@aepfle.de>

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@aepfle.de>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_fcopy.c |   36 ++++++++++++++++++++----------------
 1 files 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);
 }
-- 
1.7.4.1


  parent reply	other threads:[~2015-09-16  0:13 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-16  0:37 [PATCH 0/5] Drivers: hv: Miscellaneous fixes K. Y. Srinivasan
2015-09-16  0:37 ` [PATCH 1/5] Drivers: hv: vmbus: fix init_vp_index() for reloading hv_netvsc K. Y. Srinivasan
2015-09-16  0:37   ` [PATCH 2/5] hv: add helpers to handle hv_util device state K. Y. Srinivasan
2015-09-21  5:25     ` Greg KH
2015-09-21 10:26       ` Olaf Hering
2015-09-21 11:25         ` Vitaly Kuznetsov
2015-09-21 12:17           ` Olaf Hering
2015-09-21 13:37             ` Vitaly Kuznetsov
2015-09-21 16:45             ` KY Srinivasan
2015-09-21 16:34         ` KY Srinivasan
2015-09-21 16:43           ` Greg KH
2015-09-21 17:00             ` KY Srinivasan
2015-09-16  0:37   ` K. Y. Srinivasan [this message]
2015-09-16  0:37   ` [PATCH 4/5] hv: kvp: use wrappers to propaigate state K. Y. Srinivasan
2015-09-21  5:26     ` Greg KH
2015-09-21 10:18       ` Olaf Hering
2015-09-21 16:31         ` KY Srinivasan
2015-09-21 16:16       ` KY Srinivasan
2015-09-16  0:37   ` [PATCH 5/5] hv: vss: use wrappers to propagate state K. Y. Srinivasan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1442363874-22508-3-git-send-email-kys@microsoft.com \
    --to=kys@microsoft.com \
    --cc=apw@canonical.com \
    --cc=devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olaf@aepfle.de \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox