From patchwork Tue Dec 19 19:30:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Kim, Dongwon" X-Patchwork-Id: 10124163 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 40D4E6019C for ; Tue, 19 Dec 2017 19:41:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E11E5293F6 for ; Tue, 19 Dec 2017 19:41:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D623B294E3; Tue, 19 Dec 2017 19:41:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 73F71293F6 for ; Tue, 19 Dec 2017 19:41:01 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EA5F36E342; Tue, 19 Dec 2017 19:37:11 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id B91516E363 for ; Tue, 19 Dec 2017 19:37:05 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Dec 2017 11:37:05 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,428,1508828400"; d="scan'208";a="4018667" Received: from downor-z87x-ud5h.fm.intel.com ([10.1.122.11]) by orsmga007.jf.intel.com with ESMTP; 19 Dec 2017 11:37:05 -0800 From: Dongwon Kim To: linux-kernel@vger.kernel.org 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 Cc: xen-devel@lists.xenproject.org, mateuszx.potrola@intel.com, dri-devel@lists.freedesktop.org, dongwon.kim@intel.com X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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,