public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dongwon Kim <dongwon.kim@intel.com>
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	[thread overview]
Message-ID: <1513711816-2618-51-git-send-email-dongwon.kim@intel.com> (raw)
In-Reply-To: <1513711816-2618-1-git-send-email-dongwon.kim@intel.com>

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 <dongwon.kim@intel.com>
---
 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

  parent reply	other threads:[~2017-12-19 19:39 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-19 19:29 [RFC PATCH 01/60] hyper_dmabuf: initial working version of hyper_dmabuf drv Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 02/60] hyper_dmabuf: added a doc for hyper_dmabuf sharing Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 03/60] hyper_dmabuf: re-use dma_buf previously exported if exist Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 04/60] hyper_dmabuf: new index, k for pointing a right n-th page Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 05/60] hyper_dmabuf: skip creating a comm ch if exist for the VM Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 06/60] hyper_dmabuf: map shared pages only once when importing Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 07/60] hyper_dmabuf: message parsing done via workqueue Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 08/60] hyper_dmabuf: automatic comm channel initialization using xenstore Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 09/60] hyper_dmabuf: indirect DMA_BUF synchronization via shadowing Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 10/60] hyper_dmabuf: make sure to free memory to prevent leak Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 11/60] hyper_dmabuf: check stack before unmapping/detaching shadow DMA_BUF Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 12/60] hyper_dmabuf: two different unexporting mechanisms Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 13/60] hyper_dmabuf: postponing cleanup of hyper_DMABUF Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 14/60] hyper_dmabuf: clean-up process based on file->f_count Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 15/60] hyper_dmabuf: reusing previously released hyper_dmabuf_id Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 16/60] hyper_dmabuf: define hypervisor specific backend API Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 17/60] hyper_dmabuf: use dynamic debug macros for logging Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 18/60] hyper_dmabuf: reset comm channel when one end has disconnected Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 19/60] hyper_dmabuf: fix the case with sharing a buffer with 2 pages Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 20/60] hyper_dmabuf: optimized loop with less condition check Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 21/60] hyper_dmabuf: exposing drv information using sysfs Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 22/60] hyper_dmabuf: configure license Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 23/60] hyper_dmabuf: use CONFIG_HYPER_DMABUF_XEN instead of CONFIG_XEN Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 24/60] hyper_dmabuf: waits for resp only if WAIT_AFTER_SYNC_REQ == 1 Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 25/60] hyper_dmabuf: introduced delayed unexport Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 26/60] hyper_dmabuf: add mutexes to prevent several race conditions Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 27/60] hyper_dmabuf: use proper error codes Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 28/60] hyper_dmabuf: address several synchronization issues Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 29/60] hyper_dmabuf: make sure to release allocated buffers when exiting Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 30/60] hyper_dmabuf: free already mapped pages when error happens Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 31/60] hyper_dmabuf: built-in compilation option Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 32/60] hyper_dmabuf: make all shared pages read-only Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 33/60] hyper_dmabuf: error checking on the result of dma_buf_map_attachment Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 34/60] hyper_dmabuf: extend DMA bitmask to 64-bits Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 35/60] hyper_dmabuf: 128bit hyper_dmabuf_id with random keys Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 36/60] hyper_dmabuf: error handling when share_pages fails Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 37/60] hyper_dmabuf: implementation of query ioctl Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 38/60] hyper_dmabuf: preventing self exporting of dma_buf Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 39/60] hyper_dmabuf: correcting DMA-BUF clean-up order Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 40/60] hyper_dmabuf: do not use 'private' as field name Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 41/60] hyper_dmabuf: re-organize driver source Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 42/60] hyper_dmabuf: always generate a new random keys Dongwon Kim
2017-12-19 19:29 ` [RFC PATCH 43/60] hyper_dmabuf: fixes on memory leaks in various places Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 44/60] hyper_dmabuf: proper handling of sgt_info->priv Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 45/60] hyper_dmabuf: adding poll/read for event generation Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 46/60] hyper_dmabuf: delay auto initialization of comm_env Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 47/60] hyper_dmabuf: fix issues with event-polling Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 48/60] hyper_dmabuf: add query items for buffer private info Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 49/60] hyper_dmabuf: general clean-up and fixes Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 50/60] hyper_dmabuf: fix styling err and warns caught by checkpatch.pl Dongwon Kim
2017-12-19 19:30 ` Dongwon Kim [this message]
2017-12-19 19:30 ` [RFC PATCH 52/60] hyper_dmabuf: remove prefix 'hyper_dmabuf' from static func and backend APIs Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 53/60] hyper_dmabuf: define fastpath_export for exporting existing buffer Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 54/60] hyper_dmabuf: 'backend_ops' reduced to 'bknd_ops' and 'ops' to 'bknd_ops' Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 55/60] hyper_dmabuf: fixed wrong send_req call Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 56/60] hyper_dmabuf: add initialization and cleanup to bknd_ops Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 57/60] hyper_dmabuf: change type of ref to shared pages to unsigned long Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 58/60] hyper_dmabuf: move device node out of /dev/xen/ Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 59/60] hyper_dmabuf: freeing hy_drv_priv when drv init fails (v2) Dongwon Kim
2017-12-19 19:30 ` [RFC PATCH 60/60] hyper_dmabuf: move hyper_dmabuf to under drivers/dma-buf/ Dongwon Kim
2017-12-19 23:27 ` [RFC PATCH 01/60] hyper_dmabuf: initial working version of hyper_dmabuf drv Dongwon Kim
2017-12-20  8:17   ` [Xen-devel] " Juergen Gross
2018-01-10 23:21     ` Dongwon Kim
2017-12-20  8:38   ` Oleksandr Andrushchenko
2018-01-10 23:14     ` Dongwon Kim
2017-12-20  9:59   ` Daniel Vetter
2017-12-26 18:19     ` Matt Roper
2017-12-29 13:03       ` Tomeu Vizoso
2018-01-10 23:13     ` Dongwon Kim
2018-02-15  1:34 ` Dongwon Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1513711816-2618-51-git-send-email-dongwon.kim@intel.com \
    --to=dongwon.kim@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mateuszx.potrola@intel.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox