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);