[RFC PATCH 07/60] hyper_dmabuf: message parsing done via workqueue

[Dongwon Kim <dongwon.kim@intel.com>]

Use workqueue mechanism to delay message parsing done
after exiting from ISR to reduce ISR execution time.

Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c        |  13 ++
 drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h        |   5 +
 drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c      |   4 +-
 drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c        | 155 ++++++++++++++-------
 .../xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c   |  75 ++++------
 .../xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h   |   7 -
 6 files changed, 152 insertions(+), 107 deletions(-)

diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c
index 0698327..70b4878 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c
@@ -1,5 +1,8 @@
 #include <linux/init.h>       /* module_init, module_exit */
 #include <linux/module.h> /* version info, MODULE_LICENSE, MODULE_AUTHOR, printk() */
+#include <linux/workqueue.h>
+#include <xen/grant_table.h>
+#include "hyper_dmabuf_drv.h"
 #include "hyper_dmabuf_conf.h"
 #include "hyper_dmabuf_list.h"
 #include "xen/hyper_dmabuf_xen_comm_list.h"
@@ -10,6 +13,8 @@ MODULE_AUTHOR("IOTG-PED, INTEL");
 int register_device(void);
 int unregister_device(void);
 
+struct hyper_dmabuf_private hyper_dmabuf_private;
+
 /*===============================================================================================*/
 static int hyper_dmabuf_drv_init(void)
 {
@@ -24,6 +29,10 @@ static int hyper_dmabuf_drv_init(void)
 
 	printk( KERN_NOTICE "initializing database for imported/exported dmabufs\n");
 
+	/* device structure initialization */
+	/* currently only does work-queue initialization */
+	hyper_dmabuf_private.work_queue = create_workqueue("hyper_dmabuf_wqueue");
+
 	ret = hyper_dmabuf_table_init();
 	if (ret < 0) {
 		return -EINVAL;
@@ -45,6 +54,10 @@ static void hyper_dmabuf_drv_exit(void)
 	hyper_dmabuf_table_destroy();
 	hyper_dmabuf_ring_table_init();
 
+	/* destroy workqueue */
+	if (hyper_dmabuf_private.work_queue)
+		destroy_workqueue(hyper_dmabuf_private.work_queue);
+
 	printk( KERN_NOTICE "dma_buf-src_sink model: Exiting" );
 	unregister_device();
 }
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h
index 2dad9a6..6145d29 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.h
@@ -1,6 +1,11 @@
 #ifndef __LINUX_PUBLIC_HYPER_DMABUF_DRV_H__
 #define __LINUX_PUBLIC_HYPER_DMABUF_DRV_H__
 
+struct hyper_dmabuf_private {
+        struct device *device;
+	struct workqueue_struct *work_queue;
+};
+
 typedef int (*hyper_dmabuf_ioctl_t)(void *data);
 
 struct hyper_dmabuf_ioctl_desc {
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
index af94359..e4d8316 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c
@@ -15,9 +15,7 @@
 #include "xen/hyper_dmabuf_xen_comm_list.h"
 #include "hyper_dmabuf_msg.h"
 
-struct hyper_dmabuf_private {
-	struct device *device;
-} hyper_dmabuf_private;
+extern struct hyper_dmabuf_private hyper_dmabuf_private;
 
 static uint32_t hyper_dmabuf_id_gen(void) {
 	/* TODO: add proper implementation */
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c
index 3237e50..0166e61 100644
--- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c
+++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c
@@ -3,12 +3,23 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/dma-buf.h>
+#include <xen/grant_table.h>
+#include <linux/workqueue.h>
+#include "hyper_dmabuf_drv.h"
 #include "hyper_dmabuf_imp.h"
 //#include "hyper_dmabuf_remote_sync.h"
 #include "xen/hyper_dmabuf_xen_comm.h"
 #include "hyper_dmabuf_msg.h"
 #include "hyper_dmabuf_list.h"
 
+extern struct hyper_dmabuf_private hyper_dmabuf_private;
+
+struct cmd_process {
+	struct work_struct work;
+	struct hyper_dmabuf_ring_rq *rq;
+	int domid;
+};
+
 void hyper_dmabuf_create_request(struct hyper_dmabuf_ring_rq *request,
 				        enum hyper_dmabuf_command command, int *operands)
 {
@@ -71,18 +82,17 @@ void hyper_dmabuf_create_request(struct hyper_dmabuf_ring_rq *request,
 	}
 }
 
-int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_ring_rq *req)
+void cmd_process_work(struct work_struct *work)
 {
-	uint32_t i, ret;
 	struct hyper_dmabuf_imported_sgt_info *imported_sgt_info;
-	struct hyper_dmabuf_sgt_info *sgt_info;
-
-	/* make sure req is not NULL (may not be needed) */
-	if (!req) {
-		return -EINVAL;
-	}
+        struct hyper_dmabuf_sgt_info *sgt_info;
+	struct cmd_process *proc = container_of(work, struct cmd_process, work);
+	struct hyper_dmabuf_ring_rq *req;
+	int domid;
+	int i;
 
-	req->status = HYPER_DMABUF_REQ_PROCESSED;
+	req = proc->rq;
+	domid = proc->domid;
 
 	switch (req->command) {
 	case HYPER_DMABUF_EXPORT:
@@ -115,33 +125,6 @@ int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_ring_rq *req)
 		hyper_dmabuf_register_imported(imported_sgt_info);
 		break;
 
-	case HYPER_DMABUF_DESTROY:
-		/* destroy sg_list for hyper_dmabuf_id on remote side */
-		/* command : DMABUF_DESTROY,
-		 * operands0 : hyper_dmabuf_id
-		 */
-
-		imported_sgt_info =
-			hyper_dmabuf_find_imported(req->operands[0]);
-
-		if (imported_sgt_info) {
-			hyper_dmabuf_cleanup_imported_pages(imported_sgt_info);
-
-			hyper_dmabuf_remove_imported(req->operands[0]);
-
-			/* TODO: cleanup sgt on importer side etc */
-		}
-
-		/* Notify exporter that buffer is freed and it can cleanup it */
-		req->status = HYPER_DMABUF_REQ_NEEDS_FOLLOW_UP;
-		req->command = HYPER_DMABUF_DESTROY_FINISH;
-
-#if 0 /* function is not implemented yet */
-
-		ret = hyper_dmabuf_destroy_sgt(req->hyper_dmabuf_id);
-#endif
-		break;
-
 	case HYPER_DMABUF_DESTROY_FINISH:
 		/* destroy sg_list for hyper_dmabuf_id on local side */
 		/* command : DMABUF_DESTROY_FINISH,
@@ -180,33 +163,101 @@ int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_ring_rq *req)
 		 */
 		break;
 
-	/* requesting the other side to setup another ring channel for reverse direction */
-	case HYPER_DMABUF_EXPORTER_RING_SETUP:
-		/* command: HYPER_DMABUF_EXPORTER_RING_SETUP
-		 * no operands needed */
+	case HYPER_DMABUF_IMPORTER_RING_SETUP:
+		/* command: HYPER_DMABUF_IMPORTER_RING_SETUP */
+		/* no operands needed */
+		hyper_dmabuf_importer_ringbuf_init(domid, req->operands[0], req->operands[1]);
+
+		break;
+
+	default:
+		/* shouldn't get here */
+		/* no matched command, nothing to do.. just return error */
+		break;
+	}
+
+	kfree(req);
+	kfree(proc);
+}
+
+int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_ring_rq *req)
+{
+	struct cmd_process *proc;
+	struct hyper_dmabuf_ring_rq *temp_req;
+	struct hyper_dmabuf_imported_sgt_info *imported_sgt_info;
+	int ret;
+
+	if (!req) {
+		printk("request is NULL\n");
+		return -EINVAL;
+	}
+
+	if ((req->command < HYPER_DMABUF_EXPORT) ||
+		(req->command > HYPER_DMABUF_IMPORTER_RING_SETUP)) {
+		printk("invalid command\n");
+		return -EINVAL;
+	}
+
+	req->status = HYPER_DMABUF_REQ_PROCESSED;
+
+	/* HYPER_DMABUF_EXPORTER_RING_SETUP requires immediate
+	 * follow up so can't be processed in workqueue
+	 */
+	if (req->command == HYPER_DMABUF_EXPORTER_RING_SETUP) {
 		ret = hyper_dmabuf_exporter_ringbuf_init(domid, &req->operands[0], &req->operands[1]);
 		if (ret < 0) {
 			req->status = HYPER_DMABUF_REQ_ERROR;
-			return -EINVAL;
 		}
 
 		req->status = HYPER_DMABUF_REQ_NEEDS_FOLLOW_UP;
 		req->command = HYPER_DMABUF_IMPORTER_RING_SETUP;
-		break;
 
-	case HYPER_DMABUF_IMPORTER_RING_SETUP:
-		/* command: HYPER_DMABUF_IMPORTER_RING_SETUP */
-		/* no operands needed */
-		ret = hyper_dmabuf_importer_ringbuf_init(domid, req->operands[0], req->operands[1]);
-		if (ret < 0)
-			return -EINVAL;
+		return req->command;
+	}
 
-		break;
+	/* HYPER_DMABUF_DESTROY requires immediate
+	 * follow up so can't be processed in workqueue
+	 */
+	if (req->command == HYPER_DMABUF_DESTROY) {
+		/* destroy sg_list for hyper_dmabuf_id on remote side */
+		/* command : DMABUF_DESTROY,
+		 * operands0 : hyper_dmabuf_id
+		 */
+		imported_sgt_info =
+			hyper_dmabuf_find_imported(req->operands[0]);
 
-	default:
-		/* no matched command, nothing to do.. just return error */
-		return -EINVAL;
+		if (imported_sgt_info) {
+			hyper_dmabuf_cleanup_imported_pages(imported_sgt_info);
+
+			hyper_dmabuf_remove_imported(req->operands[0]);
+
+			/* TODO: cleanup sgt on importer side etc */
+		}
+
+		/* Notify exporter that buffer is freed and it can cleanup it */
+		req->status = HYPER_DMABUF_REQ_NEEDS_FOLLOW_UP;
+		req->command = HYPER_DMABUF_DESTROY_FINISH;
+
+#if 0 /* function is not implemented yet */
+
+		ret = hyper_dmabuf_destroy_sgt(req->hyper_dmabuf_id);
+#endif
+		return req->command;
 	}
 
+	temp_req = (struct hyper_dmabuf_ring_rq *)kmalloc(sizeof(*temp_req), GFP_KERNEL);
+
+	memcpy(temp_req, req, sizeof(*temp_req));
+
+	proc = (struct cmd_process *) kcalloc(1, sizeof(struct cmd_process),
+						GFP_KERNEL);
+
+	proc->rq = temp_req;
+	proc->domid = domid;
+
+	INIT_WORK(&(proc->work), cmd_process_work);
+
+	queue_work(hyper_dmabuf_private.work_queue, &(proc->work));
+
 	return req->command;
 }
diff --git a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c
index 22f2ef0..05855ba1 100644
--- a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c
+++ b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c
@@ -14,7 +14,6 @@
 #include "../hyper_dmabuf_msg.h"
 
 static int export_req_id = 0;
-static int import_req_id = 0;
 
 int32_t hyper_dmabuf_get_domid(void)
 {
@@ -37,12 +36,6 @@ int hyper_dmabuf_next_req_id_export(void)
         return export_req_id;
 }
 
-int hyper_dmabuf_next_req_id_import(void)
-{
-        import_req_id++;
-        return import_req_id;
-}
-
 /* For now cache latast rings as global variables TODO: keep them in list*/
 static irqreturn_t hyper_dmabuf_front_ring_isr(int irq, void *dev_id);
 static irqreturn_t hyper_dmabuf_back_ring_isr(int irq, void *dev_id);
@@ -81,7 +74,8 @@ int hyper_dmabuf_exporter_ringbuf_init(int rdomain, grant_ref_t *refid, int *por
 
 	alloc_unbound.dom = DOMID_SELF;
 	alloc_unbound.remote_dom = rdomain;
-	ret = HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound, &alloc_unbound);
+	ret = HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound,
+					&alloc_unbound);
 	if (ret != 0) {
 		printk("Cannot allocate event channel\n");
 		return -EINVAL;
@@ -96,7 +90,8 @@ int hyper_dmabuf_exporter_ringbuf_init(int rdomain, grant_ref_t *refid, int *por
 		printk("Failed to setup event channel\n");
 		close.port = alloc_unbound.port;
 		HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
-		gnttab_end_foreign_access(ring_info->gref_ring, 0, virt_to_mfn(shared_ring));
+		gnttab_end_foreign_access(ring_info->gref_ring, 0,
+					virt_to_mfn(shared_ring));
 		return -EINVAL;
 	}
 
@@ -108,7 +103,8 @@ int hyper_dmabuf_exporter_ringbuf_init(int rdomain, grant_ref_t *refid, int *por
 	*refid = ring_info->gref_ring;
 	*port = ring_info->port;
 
-	printk("%s: allocated eventchannel gref %d  port: %d  irq: %d\n", __func__,
+	printk("%s: allocated eventchannel gref %d  port: %d  irq: %d\n",
+		__func__,
 		ring_info->gref_ring,
 		ring_info->port,
 		ring_info->irq);
@@ -162,8 +158,9 @@ int hyper_dmabuf_importer_ringbuf_init(int sdomain, grant_ref_t gref, int port)
 
 	BACK_RING_INIT(&ring_info->ring_back, sring, PAGE_SIZE);
 
-	ret = bind_interdomain_evtchn_to_irqhandler(sdomain, port, hyper_dmabuf_back_ring_isr, 0,
-						    NULL, (void*)ring_info);
+	ret = bind_interdomain_evtchn_to_irqhandler(sdomain, port,
+						hyper_dmabuf_back_ring_isr, 0,
+						NULL, (void*)ring_info);
 	if (ret < 0) {
 		return -EINVAL;
 	}
@@ -216,26 +213,20 @@ int hyper_dmabuf_send_request(int domain, struct hyper_dmabuf_ring_rq *req)
 	return 0;
 }
 
-/* called by interrupt (WORKQUEUE) */
-int hyper_dmabuf_send_response(struct hyper_dmabuf_ring_rp* response, int domain)
-{
-	/* as a importer and as a exporter */
-	return 0;
-}
-
 /* ISR for request from exporter (as an importer) */
-static irqreturn_t hyper_dmabuf_back_ring_isr(int irq, void *dev_id)
+static irqreturn_t hyper_dmabuf_back_ring_isr(int irq, void *info)
 {
 	RING_IDX rc, rp;
-	struct hyper_dmabuf_ring_rq request;
-	struct hyper_dmabuf_ring_rp response;
+	struct hyper_dmabuf_ring_rq req;
+	struct hyper_dmabuf_ring_rp resp;
+
 	int notify, more_to_do;
 	int ret;
-//	struct hyper_dmabuf_work *work;
 
-	struct hyper_dmabuf_ring_info_import *ring_info = (struct hyper_dmabuf_ring_info_import *)dev_id;
+	struct hyper_dmabuf_ring_info_import *ring_info;
 	struct hyper_dmabuf_back_ring *ring;
 
+	ring_info = (struct hyper_dmabuf_ring_info_import *)info;
 	ring = &ring_info->ring_back;
 
 	do {
@@ -246,22 +237,16 @@ static irqreturn_t hyper_dmabuf_back_ring_isr(int irq, void *dev_id)
 			if (RING_REQUEST_CONS_OVERFLOW(ring, rc))
 				break;
 
-			memcpy(&request, RING_GET_REQUEST(ring, rc), sizeof(request));
+			memcpy(&req, RING_GET_REQUEST(ring, rc), sizeof(req));
 			printk("Got request\n");
 			ring->req_cons = ++rc;
 
-			/* TODO: probably using linked list for multiple requests then let
-			 * a task in a workqueue to process those is better idea becuase
-			 * we do not want to stay in ISR for long.
-			 */
-			ret = hyper_dmabuf_msg_parse(ring_info->sdomain, &request);
+			ret = hyper_dmabuf_msg_parse(ring_info->sdomain, &req);
 
 			if (ret > 0) {
-				/* build response */
-				memcpy(&response, &request, sizeof(response));
-
-				/* we sent back modified request as a response.. we might just need to have request only..*/
-				memcpy(RING_GET_RESPONSE(ring, ring->rsp_prod_pvt), &response, sizeof(response));
+				memcpy(&resp, &req, sizeof(resp));
+				memcpy(RING_GET_RESPONSE(ring, ring->rsp_prod_pvt), &resp,
+							sizeof(resp));
 				ring->rsp_prod_pvt++;
 
 				RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(ring, notify);
@@ -281,15 +266,17 @@ static irqreturn_t hyper_dmabuf_back_ring_isr(int irq, void *dev_id)
 }
 
 /* ISR for responses from importer */
-static irqreturn_t hyper_dmabuf_front_ring_isr(int irq, void *dev_id)
+static irqreturn_t hyper_dmabuf_front_ring_isr(int irq, void *info)
 {
 	/* front ring only care about response from back */
-	struct hyper_dmabuf_ring_rp *response;
+	struct hyper_dmabuf_ring_rp *resp;
 	RING_IDX i, rp;
 	int more_to_do, ret;
 
-	struct hyper_dmabuf_ring_info_export *ring_info = (struct hyper_dmabuf_ring_info_export *)dev_id;
+	struct hyper_dmabuf_ring_info_export *ring_info;
 	struct hyper_dmabuf_front_ring *ring;
+
+	ring_info = (struct hyper_dmabuf_ring_info_export *)info;
 	ring = &ring_info->ring_front;
 
 	do {
@@ -298,20 +285,18 @@ static irqreturn_t hyper_dmabuf_front_ring_isr(int irq, void *dev_id)
 		for (i = ring->rsp_cons; i != rp; i++) {
 			unsigned long id;
 
-			response = RING_GET_RESPONSE(ring, i);
-			id = response->response_id;
+			resp = RING_GET_RESPONSE(ring, i);
+			id = resp->response_id;
 
-			if (response->status == HYPER_DMABUF_REQ_NEEDS_FOLLOW_UP) {
+			if (resp->status == HYPER_DMABUF_REQ_NEEDS_FOLLOW_UP) {
 				/* parsing response */
-				ret = hyper_dmabuf_msg_parse(ring_info->rdomain, (struct hyper_dmabuf_ring_rq*)response);
+				ret = hyper_dmabuf_msg_parse(ring_info->rdomain,
+							(struct hyper_dmabuf_ring_rq *)resp);
 
 				if (ret < 0) {
 					printk("getting error while parsing response\n");
 				}
-			} else if (response->status == HYPER_DMABUF_REQ_ERROR) {
-				printk("remote domain %d couldn't process request %d\n", ring_info->rdomain, response->command);
 			}
-
 		}
 
 		ring->rsp_cons = i;
diff --git a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h
index 2754917..4ad0529 100644
--- a/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h
+++ b/drivers/xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.h
@@ -36,17 +36,10 @@ struct hyper_dmabuf_ring_info_import {
         struct hyper_dmabuf_back_ring ring_back;
 };
 
-//struct hyper_dmabuf_work {
-//	hyper_dmabuf_ring_rq requrest;
-//	struct work_struct msg_parse;
-//};
-
 int32_t hyper_dmabuf_get_domid(void);
 
 int hyper_dmabuf_next_req_id_export(void);
 
-int hyper_dmabuf_next_req_id_import(void);
-
 /* exporter needs to generated info for page sharing */
 int hyper_dmabuf_exporter_ringbuf_init(int rdomain, grant_ref_t *gref, int *port);
 
-- 
2.7.4