Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753554AbdLSTjl (ORCPT ); Tue, 19 Dec 2017 14:39:41 -0500 Received: from mga01.intel.com ([192.55.52.88]:17651 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752881AbdLSThG (ORCPT ); Tue, 19 Dec 2017 14:37:06 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,428,1508828400"; d="scan'208";a="4018667" From: Dongwon Kim To: linux-kernel@vger.kernel.org Cc: dri-devel@lists.freedesktop.org, xen-devel@lists.xenproject.org, mateuszx.potrola@intel.com, dongwon.kim@intel.com Subject: [RFC PATCH 51/60] hyper_dmabuf: missing mutex_unlock and move spinlock Date: Tue, 19 Dec 2017 11:30:07 -0800 Message-Id: <1513711816-2618-51-git-send-email-dongwon.kim@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1513711816-2618-1-git-send-email-dongwon.kim@intel.com> References: <1513711816-2618-1-git-send-email-dongwon.kim@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7557 Lines: 243 Added missing mutex_unlock to make sure mutex is unlocked before returning. Also, moved spinlock lock/unlock into hyper_dmabuf_send_event and remove checking on spinlock (with assumption caller does the spinlock in advance) to make it more straight forward. This patch includes a couple of minor modifications, changing type of function calls to static and correcting some of error code. Signed-off-by: Dongwon Kim --- drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c | 38 +++++++++++++-------------- drivers/xen/hyper_dmabuf/hyper_dmabuf_event.c | 15 +++++------ drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c | 8 ++++-- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c index 023d7f4..76f57c2 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c @@ -73,7 +73,7 @@ static void hyper_dmabuf_force_free(struct exported_sgt_info *exported, } } -int hyper_dmabuf_open(struct inode *inode, struct file *filp) +static int hyper_dmabuf_open(struct inode *inode, struct file *filp) { int ret = 0; @@ -84,7 +84,7 @@ int hyper_dmabuf_open(struct inode *inode, struct file *filp) return ret; } -int hyper_dmabuf_release(struct inode *inode, struct file *filp) +static int hyper_dmabuf_release(struct inode *inode, struct file *filp) { hyper_dmabuf_foreach_exported(hyper_dmabuf_force_free, filp); @@ -93,20 +93,18 @@ int hyper_dmabuf_release(struct inode *inode, struct file *filp) #ifdef CONFIG_HYPER_DMABUF_EVENT_GEN -unsigned int hyper_dmabuf_event_poll(struct file *filp, +static unsigned int hyper_dmabuf_event_poll(struct file *filp, struct poll_table_struct *wait) { - unsigned int mask = 0; - poll_wait(filp, &hy_drv_priv->event_wait, wait); if (!list_empty(&hy_drv_priv->event_list)) - mask |= POLLIN | POLLRDNORM; + return POLLIN | POLLRDNORM; - return mask; + return 0; } -ssize_t hyper_dmabuf_event_read(struct file *filp, char __user *buffer, +static ssize_t hyper_dmabuf_event_read(struct file *filp, char __user *buffer, size_t count, loff_t *offset) { int ret; @@ -115,14 +113,14 @@ ssize_t hyper_dmabuf_event_read(struct file *filp, char __user *buffer, if (!capable(CAP_DAC_OVERRIDE)) { dev_err(hy_drv_priv->dev, "Only root can read events\n"); - return -EFAULT; + return -EPERM; } /* make sure user buffer can be written */ if (!access_ok(VERIFY_WRITE, buffer, count)) { dev_err(hy_drv_priv->dev, "User buffer can't be written.\n"); - return -EFAULT; + return -EINVAL; } ret = mutex_lock_interruptible(&hy_drv_priv->event_read_lock); @@ -143,6 +141,7 @@ ssize_t hyper_dmabuf_event_read(struct file *filp, char __user *buffer, if (!e) { if (ret) break; + if (filp->f_flags & O_NONBLOCK) { ret = -EAGAIN; break; @@ -233,7 +232,7 @@ static struct miscdevice hyper_dmabuf_miscdev = { .fops = &hyper_dmabuf_driver_fops, }; -int register_device(void) +static int register_device(void) { int ret = 0; @@ -252,7 +251,7 @@ int register_device(void) return ret; } -void unregister_device(void) +static void unregister_device(void) { dev_info(hy_drv_priv->dev, "hyper_dmabuf: unregister_device() is called\n"); @@ -269,10 +268,8 @@ static int __init hyper_dmabuf_drv_init(void) hy_drv_priv = kcalloc(1, sizeof(struct hyper_dmabuf_private), GFP_KERNEL); - if (!hy_drv_priv) { - printk(KERN_ERR "hyper_dmabuf: Failed to create drv\n"); - return -1; - } + if (!hy_drv_priv) + return -ENOMEM; ret = register_device(); if (ret < 0) @@ -291,7 +288,6 @@ static int __init hyper_dmabuf_drv_init(void) return -1; } - /* initializing mutexes and a spinlock */ mutex_init(&hy_drv_priv->lock); mutex_lock(&hy_drv_priv->lock); @@ -301,14 +297,14 @@ static int __init hyper_dmabuf_drv_init(void) dev_info(hy_drv_priv->dev, "initializing database for imported/exported dmabufs\n"); - /* device structure initialization */ - /* currently only does work-queue initialization */ hy_drv_priv->work_queue = create_workqueue("hyper_dmabuf_wqueue"); ret = hyper_dmabuf_table_init(); if (ret < 0) { dev_err(hy_drv_priv->dev, "failed to initialize table for exported/imported entries\n"); + mutex_unlock(&hy_drv_priv->lock); + kfree(hy_drv_priv); return ret; } @@ -317,6 +313,8 @@ static int __init hyper_dmabuf_drv_init(void) if (ret < 0) { dev_err(hy_drv_priv->dev, "failed to initialize sysfs\n"); + mutex_unlock(&hy_drv_priv->lock); + kfree(hy_drv_priv); return ret; } #endif @@ -338,7 +336,7 @@ static int __init hyper_dmabuf_drv_init(void) ret = hy_drv_priv->backend_ops->init_comm_env(); if (ret < 0) { dev_dbg(hy_drv_priv->dev, - "failed to initialize comm-env but it will re-attempt.\n"); + "failed to initialize comm-env.\n"); } else { hy_drv_priv->initialized = true; } diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_event.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_event.c index a4945af..ae8cb43 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_event.c +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_event.c @@ -37,11 +37,12 @@ #include "hyper_dmabuf_list.h" #include "hyper_dmabuf_event.h" -static void hyper_dmabuf_send_event_locked(struct hyper_dmabuf_event *e) +static void hyper_dmabuf_send_event(struct hyper_dmabuf_event *e) { struct hyper_dmabuf_event *oldest; + unsigned long irqflags; - assert_spin_locked(&hy_drv_priv->event_lock); + spin_lock_irqsave(&hy_drv_priv->event_lock, irqflags); /* check current number of event then if it hits the max num allowed * then remove the oldest event in the list @@ -60,6 +61,8 @@ static void hyper_dmabuf_send_event_locked(struct hyper_dmabuf_event *e) hy_drv_priv->pending++; wake_up_interruptible(&hy_drv_priv->event_wait); + + spin_unlock_irqrestore(&hy_drv_priv->event_lock, irqflags); } void hyper_dmabuf_events_release(void) @@ -89,8 +92,6 @@ int hyper_dmabuf_import_event(hyper_dmabuf_id_t hid) struct hyper_dmabuf_event *e; struct imported_sgt_info *imported; - unsigned long irqflags; - imported = hyper_dmabuf_find_imported(hid); if (!imported) { @@ -109,11 +110,7 @@ int hyper_dmabuf_import_event(hyper_dmabuf_id_t hid) e->event_data.data = (void *)imported->priv; e->event_data.hdr.size = imported->sz_priv; - spin_lock_irqsave(&hy_drv_priv->event_lock, irqflags); - - hyper_dmabuf_send_event_locked(e); - - spin_unlock_irqrestore(&hy_drv_priv->event_lock, irqflags); + hyper_dmabuf_send_event(e); dev_dbg(hy_drv_priv->dev, "event number = %d :", hy_drv_priv->pending); diff --git a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c index f9040ed..195cede 100644 --- a/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c +++ b/drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c @@ -441,8 +441,10 @@ static int hyper_dmabuf_export_fd_ioctl(struct file *filp, void *data) req = kcalloc(1, sizeof(*req), GFP_KERNEL); - if (!req) + if (!req) { + mutex_unlock(&hy_drv_priv->lock); return -ENOMEM; + } hyper_dmabuf_create_req(req, HYPER_DMABUF_EXPORT_FD, &op[0]); @@ -509,8 +511,10 @@ static int hyper_dmabuf_export_fd_ioctl(struct file *filp, void *data) req = kcalloc(1, sizeof(*req), GFP_KERNEL); - if (!req) + if (!req) { + mutex_unlock(&hy_drv_priv->lock); return -ENOMEM; + } hyper_dmabuf_create_req(req, HYPER_DMABUF_EXPORT_FD_FAILED, -- 2.7.4