From: Mateusz Polrola mateuszx.potrola@intel.com
It is required to release all allocated buffers when the application crashes. With the change, hyper_dmabuf_sgt_info includes file pointers for the driver. If it's released unexpectedly, the driver is now unexporting all already-exported buffers to prevent memory leak.
In case there are multiple applications exporting same buffer to another VM, unexporting is not started when one of those crashes. Actual unexporting is invoked only if the last application that exported the buffer is crashed or finished via "emergency-unexport" routine, that is executed automatically when all of file pointers opened for accessing hyper_dmabuf driver are closed.
Signed-off-by: Dongwon Kim dongwon.kim@intel.com --- drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c | 6 ++- drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c | 73 ++++++++++++++++++-------- drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.h | 2 +- drivers/xen/hyper_dmabuf/hyper_dmabuf_list.c | 14 +++++ drivers/xen/hyper_dmabuf/hyper_dmabuf_list.h | 4 ++ drivers/xen/hyper_dmabuf/hyper_dmabuf_struct.h | 7 +++ 6 files changed, 81 insertions(+), 25 deletions(-)
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c index 44a9139..a12d4dc 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c @@ -54,7 +54,7 @@ static int __init hyper_dmabuf_drv_init(void) { int ret = 0;
- printk( KERN_NOTICE "hyper_dmabuf_starting: Initialization started" ); + printk( KERN_NOTICE "hyper_dmabuf_starting: Initialization started\n");
mutex_init(&hyper_dmabuf_private.lock);
@@ -122,7 +122,9 @@ static void hyper_dmabuf_drv_exit(void) if (hyper_dmabuf_private.id_queue) destroy_reusable_list();
- printk( KERN_NOTICE "dma_buf-src_sink model: Exiting" ); + dev_info(hyper_dmabuf_private.device, + "hyper_dmabuf driver: Exiting\n"); + unregister_device(); } /*===============================================================================================*/ diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c index 58b115a..fa700f2 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c @@ -47,7 +47,7 @@
extern struct hyper_dmabuf_private hyper_dmabuf_private;
-static int hyper_dmabuf_tx_ch_setup(void *data) +static int hyper_dmabuf_tx_ch_setup(struct file *filp, void *data) { struct ioctl_hyper_dmabuf_tx_ch_setup *tx_ch_attr; struct hyper_dmabuf_backend_ops *ops = hyper_dmabuf_private.backend_ops; @@ -64,7 +64,7 @@ static int hyper_dmabuf_tx_ch_setup(void *data) return ret; }
-static int hyper_dmabuf_rx_ch_setup(void *data) +static int hyper_dmabuf_rx_ch_setup(struct file *filp, void *data) { struct ioctl_hyper_dmabuf_rx_ch_setup *rx_ch_attr; struct hyper_dmabuf_backend_ops *ops = hyper_dmabuf_private.backend_ops; @@ -82,7 +82,7 @@ static int hyper_dmabuf_rx_ch_setup(void *data) return ret; }
-static int hyper_dmabuf_export_remote(void *data) +static int hyper_dmabuf_export_remote(struct file *filp, void *data) { struct ioctl_hyper_dmabuf_export_remote *export_remote_attr; struct hyper_dmabuf_backend_ops *ops = hyper_dmabuf_private.backend_ops; @@ -227,6 +227,8 @@ static int hyper_dmabuf_export_remote(void *data) kfree(page_info->pages); kfree(page_info);
+ sgt_info->filp = filp; + return ret;
fail_send_request: @@ -248,7 +250,7 @@ static int hyper_dmabuf_export_remote(void *data) return ret; }
-static int hyper_dmabuf_export_fd_ioctl(void *data) +static int hyper_dmabuf_export_fd_ioctl(struct file *filp, void *data) { struct ioctl_hyper_dmabuf_export_fd *export_fd_attr; struct hyper_dmabuf_backend_ops *ops = hyper_dmabuf_private.backend_ops; @@ -411,7 +413,7 @@ static void hyper_dmabuf_delayed_unexport(struct work_struct *work)
/* Schedules unexport of dmabuf. */ -static int hyper_dmabuf_unexport(void *data) +static int hyper_dmabuf_unexport(struct file *filp, void *data) { struct ioctl_hyper_dmabuf_unexport *unexport_attr; struct hyper_dmabuf_sgt_info *sgt_info; @@ -448,7 +450,7 @@ static int hyper_dmabuf_unexport(void *data) return 0; }
-static int hyper_dmabuf_query(void *data) +static int hyper_dmabuf_query(struct file *filp, void *data) { struct ioctl_hyper_dmabuf_query *query_attr; struct hyper_dmabuf_sgt_info *sgt_info; @@ -558,7 +560,7 @@ static long hyper_dmabuf_ioctl(struct file *filp, return -EFAULT; }
- ret = func(kdata); + ret = func(filp, kdata);
if (copy_to_user((void __user *)param, kdata, _IOC_SIZE(cmd)) != 0) { dev_err(hyper_dmabuf_private.device, "failed to copy to user arguments\n"); @@ -570,14 +572,49 @@ static long hyper_dmabuf_ioctl(struct file *filp, return ret; }
-struct device_info { - int curr_domain; -}; +int hyper_dmabuf_open(struct inode *inode, struct file *filp) +{ + /* Do not allow exclusive open */ + if (filp->f_flags & O_EXCL) + return -EBUSY; + + return 0; +} + +static void hyper_dmabuf_emergency_release(struct hyper_dmabuf_sgt_info* sgt_info, + void *attr) +{ + struct ioctl_hyper_dmabuf_unexport unexport_attr; + struct file *filp = (struct file*) attr; + + if (!filp || !sgt_info) + return; + + if (sgt_info->filp == filp) { + dev_dbg(hyper_dmabuf_private.device, + "Executing emergency release of buffer %d\n", + sgt_info->hyper_dmabuf_id); + + unexport_attr.hyper_dmabuf_id = sgt_info->hyper_dmabuf_id; + unexport_attr.delay_ms = 0; + + hyper_dmabuf_unexport(filp, &unexport_attr); + } +} + +int hyper_dmabuf_release(struct inode *inode, struct file *filp) +{ + hyper_dmabuf_foreach_exported(hyper_dmabuf_emergency_release, filp); + + return 0; +}
/*===============================================================================================*/ static struct file_operations hyper_dmabuf_driver_fops = { .owner = THIS_MODULE, + .open = hyper_dmabuf_open, + .release = hyper_dmabuf_release, .unlocked_ioctl = hyper_dmabuf_ioctl, };
@@ -597,7 +634,7 @@ int register_device(void) ret = misc_register(&hyper_dmabuf_miscdev);
if (ret) { - printk(KERN_WARNING "hyper_dmabuf: driver can't be registered\n"); + printk(KERN_ERR "hyper_dmabuf: driver can't be registered\n"); return ret; }
@@ -606,22 +643,14 @@ int register_device(void) /* TODO: Check if there is a different way to initialize dma mask nicely */ dma_coerce_mask_and_coherent(hyper_dmabuf_private.device, 0xFFFFFFFF);
- /* TODO find a way to provide parameters for below function or move that to ioctl */ -/* err = bind_interdomain_evtchn_to_irqhandler(rdomain, evtchn, - src_sink_isr, PORT_NUM, "remote_domain", &info); - if (err < 0) { - printk("hyper_dmabuf: can't register interrupt handlers\n"); - return -EFAULT; - } - - info.irq = err; -*/ return ret; }
/*-----------------------------------------------------------------------------------------------*/ void unregister_device(void) { - printk( KERN_NOTICE "hyper_dmabuf: unregister_device() is called" ); + dev_info(hyper_dmabuf_private.device, + "hyper_dmabuf: unregister_device() is called\n"); + misc_deregister(&hyper_dmabuf_miscdev); } diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.h b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.h index 8355e30..ebfbb84 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.h +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.h @@ -25,7 +25,7 @@ #ifndef __HYPER_DMABUF_IOCTL_H__ #define __HYPER_DMABUF_IOCTL_H__
-typedef int (*hyper_dmabuf_ioctl_t)(void *data); +typedef int (*hyper_dmabuf_ioctl_t)(struct file *filp, void *data);
struct hyper_dmabuf_ioctl_desc { unsigned int cmd; diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_list.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_list.c index 2cb4bb4..c1285eb 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_list.c +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_list.c @@ -221,3 +221,17 @@ int hyper_dmabuf_remove_imported(int id)
return -ENOENT; } + +void hyper_dmabuf_foreach_exported( + void (*func)(struct hyper_dmabuf_sgt_info *, void *attr), + void *attr) +{ + struct hyper_dmabuf_info_entry_exported *info_entry; + struct hlist_node *tmp; + int bkt; + + hash_for_each_safe(hyper_dmabuf_hash_exported, bkt, tmp, + info_entry, node) { + func(info_entry->info, attr); + } +} diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_list.h b/drivers/xen/hyper_dmabuf/hyper_dmabuf_list.h index 35dc722..925b0d1 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_list.h +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_list.h @@ -61,6 +61,10 @@ int hyper_dmabuf_remove_exported(int id);
int hyper_dmabuf_remove_imported(int id);
+void hyper_dmabuf_foreach_exported( + void (*func)(struct hyper_dmabuf_sgt_info *, void *attr), + void *attr); + int hyper_dmabuf_register_sysfs(struct device *dev); int hyper_dmabuf_unregister_sysfs(struct device *dev);
diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_struct.h b/drivers/xen/hyper_dmabuf/hyper_dmabuf_struct.h index a41fd0a..9952b3f 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_struct.h +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_struct.h @@ -80,6 +80,13 @@ struct hyper_dmabuf_sgt_info { void *refs_info; /* hypervisor-specific info for the references */ struct delayed_work unexport_work; bool unexport_scheduled; + /* owner of buffer + * TODO: that is naiive as buffer may be reused by + * another userspace app, so here list of struct file should be kept + * and emergency unexport should be executed only after last of buffer + * uses releases hyper_dmabuf device + */ + struct file *filp; int private[4]; /* device specific info (e.g. image's meta info?) */ };