Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1314183 > unrolled thread

[PATCH 00/11] sync framework de-staging: part 1 - clean up

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/11] sync framework de-staging: part 1 - clean up Gustavo Padovan <gustavo@padovan.org> - 2016-01-21 14:00 +0100
    [PATCH 10/11] staging/android: remove pointless sync_timeline_signal at destroy phase Gustavo Padovan <gustavo@padovan.org> - 2016-01-21 14:00 +0100
    [PATCH 05/11] staging/android: move SW_SYNC_USER to a debugfs file Gustavo Padovan <gustavo@padovan.org> - 2016-01-21 14:00 +0100
    Re: [PATCH 00/11] sync framework de-staging: part 1 - clean up Maarten Lankhorst <maarten.lankhorst@linux.intel.com> - 2016-01-25 12:30 +0100

#1314183 — [PATCH 00/11] sync framework de-staging: part 1 - clean up

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Hi,

The following patches are some clean ups on the sync framework before
we start the actual de-staging. The main changes here are the move of
SW_SYNC_USER to debugfs. Removal of struct sync_pt in favor of direct
use of struct fence. And the rename of sync_fence to sync_file, a name
to better reflect what it is for, a struct that connects struct fence(s)
to a file.

Please review. Thanks.

Gustavo Padovan (11):
  staging/android: fix sync framework documentation
  staging/android: sync: remove interfaces that are not used
  staging/android: remove not used sync_timeline ops
  staging/android: create a 'sync' dir for debugfs information
  staging/android: move SW_SYNC_USER to a debugfs file
  staging/android: rename sync_fence to sync_file
  staging/android: rename 'sync_pt' to 'fence' in struct sync_fence_cb
  staging/android: remove struct sync_pt
  staging/android: remove unused var from sync_timeline_signal()
  staging/android: remove pointless sync_timeline_signal at destroy
    phase
  staging/android: remove sync_fence_create_dma()

 drivers/staging/android/Kconfig      |   9 -
 drivers/staging/android/sw_sync.c    | 181 ++--------------
 drivers/staging/android/sw_sync.h    |   8 +-
 drivers/staging/android/sync.c       | 406 ++++++++++++++---------------------
 drivers/staging/android/sync.h       | 237 ++++++--------------
 drivers/staging/android/sync_debug.c | 228 +++++++++++++++-----
 drivers/staging/android/trace/sync.h |  24 +--
 drivers/staging/android/uapi/sync.h  |  14 +-
 include/linux/fence.h                |   2 +
 9 files changed, 438 insertions(+), 671 deletions(-)

-- 
2.5.0

[toc] | [next] | [standalone]

#1314184 — [PATCH 10/11] staging/android: remove pointless sync_timeline_signal at destroy phase

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

All changes to timeline value come through the user via
sync_timeline_signal() calls. When sync_timeline_destroy() is called no
changes on timeline->value happens hence call sync_timeline_signal() with
no increment is pointless.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sync.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 9ec55ef..b9f167f 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -90,10 +90,6 @@ void sync_timeline_destroy(struct sync_timeline *obj)
 	 */
 	smp_wmb();
 
-	/*
-	 * signal any children that their parent is going away.
-	 */
-	sync_timeline_signal(obj);
 	sync_timeline_put(obj);
 }
 EXPORT_SYMBOL(sync_timeline_destroy);
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1314185 — [PATCH 05/11] staging/android: move SW_SYNC_USER to a debugfs file

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

This remove CONFIG_SW_SYNC_USER and instead compile the sw_sync file into
debugpfs under <debugfs>/sync/sw_sync.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/Kconfig      |   9 ---
 drivers/staging/android/sw_sync.c    | 129 -----------------------------------
 drivers/staging/android/sync_debug.c | 118 +++++++++++++++++++++++++++++++-
 3 files changed, 117 insertions(+), 139 deletions(-)

diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
index 42b1512..bd90d20 100644
--- a/drivers/staging/android/Kconfig
+++ b/drivers/staging/android/Kconfig
@@ -57,15 +57,6 @@ config SW_SYNC
 	  synchronization.  Useful when there is no hardware primitive backing
 	  the synchronization.
 
-config SW_SYNC_USER
-	bool "Userspace API for SW_SYNC"
-	default n
-	depends on SW_SYNC
-	---help---
-	  Provides a user space API to the sw sync object.
-	  *WARNING* improper use of this can result in deadlocking kernel
-	  drivers from userspace.
-
 source "drivers/staging/android/ion/Kconfig"
 
 endif # if ANDROID
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index 566dcdc..f491dbc 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -102,132 +102,3 @@ void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc)
 	sync_timeline_signal(&obj->obj);
 }
 EXPORT_SYMBOL(sw_sync_timeline_inc);
-
-#ifdef CONFIG_SW_SYNC_USER
-/* *WARNING*
- *
- * improper use of this can result in deadlocking kernel drivers from userspace.
- */
-
-/* opening sw_sync create a new sync obj */
-static int sw_sync_open(struct inode *inode, struct file *file)
-{
-	struct sw_sync_timeline *obj;
-	char task_comm[TASK_COMM_LEN];
-
-	get_task_comm(task_comm, current);
-
-	obj = sw_sync_timeline_create(task_comm);
-	if (!obj)
-		return -ENOMEM;
-
-	file->private_data = obj;
-
-	return 0;
-}
-
-static int sw_sync_release(struct inode *inode, struct file *file)
-{
-	struct sw_sync_timeline *obj = file->private_data;
-
-	sync_timeline_destroy(&obj->obj);
-	return 0;
-}
-
-static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj,
-				       unsigned long arg)
-{
-	int fd = get_unused_fd_flags(O_CLOEXEC);
-	int err;
-	struct sync_pt *pt;
-	struct sync_fence *fence;
-	struct sw_sync_create_fence_data data;
-
-	if (fd < 0)
-		return fd;
-
-	if (copy_from_user(&data, (void __user *)arg, sizeof(data))) {
-		err = -EFAULT;
-		goto err;
-	}
-
-	pt = sw_sync_pt_create(obj, data.value);
-	if (!pt) {
-		err = -ENOMEM;
-		goto err;
-	}
-
-	data.name[sizeof(data.name) - 1] = '\0';
-	fence = sync_fence_create(data.name, pt);
-	if (!fence) {
-		sync_pt_free(pt);
-		err = -ENOMEM;
-		goto err;
-	}
-
-	data.fence = fd;
-	if (copy_to_user((void __user *)arg, &data, sizeof(data))) {
-		sync_fence_put(fence);
-		err = -EFAULT;
-		goto err;
-	}
-
-	sync_fence_install(fence, fd);
-
-	return 0;
-
-err:
-	put_unused_fd(fd);
-	return err;
-}
-
-static long sw_sync_ioctl_inc(struct sw_sync_timeline *obj, unsigned long arg)
-{
-	u32 value;
-
-	if (copy_from_user(&value, (void __user *)arg, sizeof(value)))
-		return -EFAULT;
-
-	sw_sync_timeline_inc(obj, value);
-
-	return 0;
-}
-
-static long sw_sync_ioctl(struct file *file, unsigned int cmd,
-			  unsigned long arg)
-{
-	struct sw_sync_timeline *obj = file->private_data;
-
-	switch (cmd) {
-	case SW_SYNC_IOC_CREATE_FENCE:
-		return sw_sync_ioctl_create_fence(obj, arg);
-
-	case SW_SYNC_IOC_INC:
-		return sw_sync_ioctl_inc(obj, arg);
-
-	default:
-		return -ENOTTY;
-	}
-}
-
-static const struct file_operations sw_sync_fops = {
-	.owner = THIS_MODULE,
-	.open = sw_sync_open,
-	.release = sw_sync_release,
-	.unlocked_ioctl = sw_sync_ioctl,
-	.compat_ioctl = sw_sync_ioctl,
-};
-
-static struct miscdevice sw_sync_dev = {
-	.minor	= MISC_DYNAMIC_MINOR,
-	.name	= "sw_sync",
-	.fops	= &sw_sync_fops,
-};
-
-static int __init sw_sync_device_init(void)
-{
-	return misc_register(&sw_sync_dev);
-}
-device_initcall(sw_sync_device_init);
-
-#endif /* CONFIG_SW_SYNC_USER */
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c
index ca35f97..ae08fc1 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -27,7 +27,7 @@
 #include <linux/uaccess.h>
 #include <linux/anon_inodes.h>
 #include <linux/time64.h>
-#include "sync.h"
+#include "sw_sync.h"
 
 #ifdef CONFIG_DEBUG_FS
 
@@ -208,11 +208,127 @@ static const struct file_operations sync_info_debugfs_fops = {
 	.release        = single_release,
 };
 
+/*
+ * *WARNING*
+ *
+ * improper use of this can result in deadlocking kernel drivers from userspace.
+ */
+
+/* opening sw_sync create a new sync obj */
+static int sw_sync_debugfs_open(struct inode *inode, struct file *file)
+{
+	struct sw_sync_timeline *obj;
+	char task_comm[TASK_COMM_LEN];
+
+	get_task_comm(task_comm, current);
+
+	obj = sw_sync_timeline_create(task_comm);
+	if (!obj)
+		return -ENOMEM;
+
+	file->private_data = obj;
+
+	return 0;
+}
+
+static int sw_sync_debugfs_release(struct inode *inode, struct file *file)
+{
+	struct sw_sync_timeline *obj = file->private_data;
+
+	sync_timeline_destroy(&obj->obj);
+	return 0;
+}
+
+static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj,
+				       unsigned long arg)
+{
+	int fd = get_unused_fd_flags(O_CLOEXEC);
+	int err;
+	struct sync_pt *pt;
+	struct sync_fence *fence;
+	struct sw_sync_create_fence_data data;
+
+	if (fd < 0)
+		return fd;
+
+	if (copy_from_user(&data, (void __user *)arg, sizeof(data))) {
+		err = -EFAULT;
+		goto err;
+	}
+
+	pt = sw_sync_pt_create(obj, data.value);
+	if (!pt) {
+		err = -ENOMEM;
+		goto err;
+	}
+
+	data.name[sizeof(data.name) - 1] = '\0';
+	fence = sync_fence_create(data.name, pt);
+	if (!fence) {
+		sync_pt_free(pt);
+		err = -ENOMEM;
+		goto err;
+	}
+
+	data.fence = fd;
+	if (copy_to_user((void __user *)arg, &data, sizeof(data))) {
+		sync_fence_put(fence);
+		err = -EFAULT;
+		goto err;
+	}
+
+	sync_fence_install(fence, fd);
+
+	return 0;
+
+err:
+	put_unused_fd(fd);
+	return err;
+}
+
+static long sw_sync_ioctl_inc(struct sw_sync_timeline *obj, unsigned long arg)
+{
+	u32 value;
+
+	if (copy_from_user(&value, (void __user *)arg, sizeof(value)))
+		return -EFAULT;
+
+	sw_sync_timeline_inc(obj, value);
+
+	return 0;
+}
+
+static long sw_sync_ioctl(struct file *file, unsigned int cmd,
+			  unsigned long arg)
+{
+	struct sw_sync_timeline *obj = file->private_data;
+
+	switch (cmd) {
+	case SW_SYNC_IOC_CREATE_FENCE:
+		return sw_sync_ioctl_create_fence(obj, arg);
+
+	case SW_SYNC_IOC_INC:
+		return sw_sync_ioctl_inc(obj, arg);
+
+	default:
+		return -ENOTTY;
+	}
+}
+
+static const struct file_operations sw_sync_debugfs_fops = {
+	.open           = sw_sync_debugfs_open,
+	.release        = sw_sync_debugfs_release,
+	.unlocked_ioctl = sw_sync_ioctl,
+	.compat_ioctl = sw_sync_ioctl,
+};
+
 static __init int sync_debugfs_init(void)
 {
 	dbgfs = debugfs_create_dir("sync", NULL);
 
 	debugfs_create_file("info", 0444, dbgfs, NULL, &sync_info_debugfs_fops);
+	debugfs_create_file("sw_sync", 0644, dbgfs, NULL,
+			    &sw_sync_debugfs_fops);
 
 	return 0;
 }
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1316513

Hey,

Op 21-01-16 om 13:49 schreef Gustavo Padovan:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> Hi,
>
> The following patches are some clean ups on the sync framework before
> we start the actual de-staging. The main changes here are the move of
> SW_SYNC_USER to debugfs. Removal of struct sync_pt in favor of direct
> use of struct fence. And the rename of sync_fence to sync_file, a name
> to better reflect what it is for, a struct that connects struct fence(s)
> to a file.
>
> Please review. Thanks.
>
> Gustavo Padovan (11):
>   staging/android: fix sync framework documentation
>   staging/android: sync: remove interfaces that are not used
>   staging/android: remove not used sync_timeline ops
>   staging/android: create a 'sync' dir for debugfs information
>   staging/android: move SW_SYNC_USER to a debugfs file
>   staging/android: rename sync_fence to sync_file
>   staging/android: rename 'sync_pt' to 'fence' in struct sync_fence_cb
>   staging/android: remove struct sync_pt
>   staging/android: remove unused var from sync_timeline_signal()
>   staging/android: remove pointless sync_timeline_signal at destroy
>     phase
>   staging/android: remove sync_fence_create_dma()
>
For whole series:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Renaming sync_fence to sync_file makes sense, it should hopefully reduce confusion.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web