[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

Signed-off-by: Dongwon Kim <dongwon.kim@xxxxxxxxx>
---
 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.