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


Groups > linux.kernel > #1458250 > unrolled thread

[PATCH v2 0/6] de-stage SW_SYNC validation frawework

Started byGustavo Padovan <gustavo@padovan.org>
First post2016-08-08 23:30 +0200
Last post2016-08-10 20:30 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 0/6] de-stage SW_SYNC validation frawework Gustavo Padovan <gustavo@padovan.org> - 2016-08-08 23:30 +0200
    [PATCH v2 3/6] staging/android: move trace/sync.h to sync_trace.h Gustavo Padovan <gustavo@padovan.org> - 2016-08-08 23:30 +0200
    [PATCH v2 2/6] staging/android: do not let userspace trigger WARN_ON Gustavo Padovan <gustavo@padovan.org> - 2016-08-08 23:30 +0200
    [PATCH v2 1/6] staging/android: remove doc from sw_sync Gustavo Padovan <gustavo@padovan.org> - 2016-08-08 23:30 +0200
      Re: [PATCH v2 1/6] staging/android: remove doc from sw_sync Pavel Machek <pavel@ucw.cz> - 2016-08-10 20:30 +0200

#1458250 — [PATCH v2 0/6] de-stage SW_SYNC validation frawework

FromGustavo Padovan <gustavo@padovan.org>
Date2016-08-08 23:30 +0200
Subject[PATCH v2 0/6] de-stage SW_SYNC validation frawework
Message-ID<s40hr-6Jd-5@gated-at.bofh.it>
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Hi Greg,

This is the last step in the Sync Framwork de-stage task. It de-stage
the SW_SYNC validation framework and the sync_debug info debugfs file.

The first 2 patches are clean up and improvements and the rest is preparation
to de-stage and then finally the actual de-stage.

v2: 
 - add documentation about the SW_SYNC ioctl API (comments from Pavel Machek)
 - remove for now patch to add sync_pt name to debugfs

Please review,

Gustavo

---
Gustavo Padovan (6):
  staging/android: remove doc from sw_sync
  staging/android: do not let userspace trigger WARN_ON
  staging/android: move trace/sync.h to sync_trace.h
  staging/android: prepare sw_sync files for de-staging
  staging/android: add Doc for SW_SYNC ioctl interface
  dma-buf/sw_sync: de-stage SW_SYNC

 drivers/dma-buf/Kconfig              |  13 ++
 drivers/dma-buf/Makefile             |   1 +
 drivers/dma-buf/sw_sync.c            | 349 +++++++++++++++++++++++++++++++++++
 drivers/dma-buf/sync_debug.c         | 230 +++++++++++++++++++++++
 drivers/dma-buf/sync_debug.h         |  69 +++++++
 drivers/dma-buf/sync_trace.h         |  32 ++++
 drivers/staging/android/Kconfig      |  13 --
 drivers/staging/android/Makefile     |   1 -
 drivers/staging/android/sw_sync.c    | 344 ----------------------------------
 drivers/staging/android/sync_debug.c | 230 -----------------------
 drivers/staging/android/sync_debug.h |  84 ---------
 drivers/staging/android/trace/sync.h |  32 ----
 12 files changed, 694 insertions(+), 704 deletions(-)
 create mode 100644 drivers/dma-buf/sw_sync.c
 create mode 100644 drivers/dma-buf/sync_debug.c
 create mode 100644 drivers/dma-buf/sync_debug.h
 create mode 100644 drivers/dma-buf/sync_trace.h
 delete mode 100644 drivers/staging/android/sw_sync.c
 delete mode 100644 drivers/staging/android/sync_debug.c
 delete mode 100644 drivers/staging/android/sync_debug.h
 delete mode 100644 drivers/staging/android/trace/sync.h

-- 
2.5.5

[toc] | [next] | [standalone]


#1458252 — [PATCH v2 3/6] staging/android: move trace/sync.h to sync_trace.h

FromGustavo Padovan <gustavo@padovan.org>
Date2016-08-08 23:30 +0200
Subject[PATCH v2 3/6] staging/android: move trace/sync.h to sync_trace.h
Message-ID<s40hs-6Jd-21@gated-at.bofh.it>
In reply to#1458250
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

The common behaviour for trace headers is to have them in the same folder
they are used, instead of creating a special trace/ directory.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sw_sync.c    |  2 +-
 drivers/staging/android/sync_trace.h | 32 ++++++++++++++++++++++++++++++++
 drivers/staging/android/trace/sync.h | 32 --------------------------------
 3 files changed, 33 insertions(+), 33 deletions(-)
 create mode 100644 drivers/staging/android/sync_trace.h
 delete mode 100644 drivers/staging/android/trace/sync.h

diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index ad0bb1a..745597b 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -23,7 +23,7 @@
 #include "sync_debug.h"
 
 #define CREATE_TRACE_POINTS
-#include "trace/sync.h"
+#include "sync_trace.h"
 
 struct sw_sync_create_fence_data {
 	__u32	value;
diff --git a/drivers/staging/android/sync_trace.h b/drivers/staging/android/sync_trace.h
new file mode 100644
index 0000000..ea485f7
--- /dev/null
+++ b/drivers/staging/android/sync_trace.h
@@ -0,0 +1,32 @@
+#undef TRACE_SYSTEM
+#define TRACE_INCLUDE_PATH ../../drivers/staging/android
+#define TRACE_SYSTEM sync_trace
+
+#if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SYNC_H
+
+#include "sync_debug.h"
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(sync_timeline,
+	TP_PROTO(struct sync_timeline *timeline),
+
+	TP_ARGS(timeline),
+
+	TP_STRUCT__entry(
+			__string(name, timeline->name)
+			__field(u32, value)
+	),
+
+	TP_fast_assign(
+			__assign_str(name, timeline->name);
+			__entry->value = timeline->value;
+	),
+
+	TP_printk("name=%s value=%d", __get_str(name), __entry->value)
+);
+
+#endif /* if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/drivers/staging/android/trace/sync.h b/drivers/staging/android/trace/sync.h
deleted file mode 100644
index 6b5ce96..0000000
--- a/drivers/staging/android/trace/sync.h
+++ /dev/null
@@ -1,32 +0,0 @@
-#undef TRACE_SYSTEM
-#define TRACE_INCLUDE_PATH ../../drivers/staging/android/trace
-#define TRACE_SYSTEM sync
-
-#if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ)
-#define _TRACE_SYNC_H
-
-#include "../sync_debug.h"
-#include <linux/tracepoint.h>
-
-TRACE_EVENT(sync_timeline,
-	TP_PROTO(struct sync_timeline *timeline),
-
-	TP_ARGS(timeline),
-
-	TP_STRUCT__entry(
-			__string(name, timeline->name)
-			__field(u32, value)
-	),
-
-	TP_fast_assign(
-			__assign_str(name, timeline->name);
-			__entry->value = timeline->value;
-	),
-
-	TP_printk("name=%s value=%d", __get_str(name), __entry->value)
-);
-
-#endif /* if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) */
-
-/* This part must be outside protection */
-#include <trace/define_trace.h>
-- 
2.5.5

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


#1458254 — [PATCH v2 2/6] staging/android: do not let userspace trigger WARN_ON

FromGustavo Padovan <gustavo@padovan.org>
Date2016-08-08 23:30 +0200
Subject[PATCH v2 2/6] staging/android: do not let userspace trigger WARN_ON
Message-ID<s40hs-6Jd-17@gated-at.bofh.it>
In reply to#1458250
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Closing the timeline without waiting all fences to signal is not
a critical failure, it is just bad usage from userspace so avoid
calling WARN_ON in this case.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sw_sync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index b4ae092..ad0bb1a 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -150,7 +150,7 @@ static void timeline_fence_release(struct fence *fence)
 
 	spin_lock_irqsave(fence->lock, flags);
 	list_del(&pt->child_list);
-	if (WARN_ON_ONCE(!list_empty(&pt->active_list)))
+	if (!list_empty(&pt->active_list))
 		list_del(&pt->active_list);
 	spin_unlock_irqrestore(fence->lock, flags);
 
-- 
2.5.5

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


#1458255 — [PATCH v2 1/6] staging/android: remove doc from sw_sync

FromGustavo Padovan <gustavo@padovan.org>
Date2016-08-08 23:30 +0200
Subject[PATCH v2 1/6] staging/android: remove doc from sw_sync
Message-ID<s40hs-6Jd-15@gated-at.bofh.it>
In reply to#1458250
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

SW_SYNC should never be used by other pieces of the kernel apart from
sync_debug as it is only a Sync File Validation Framework, so hide any
info to avoid confuse this with a standard kernel internal API.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sw_sync.c    | 26 --------------------------
 drivers/staging/android/sync_debug.h | 15 ---------------
 2 files changed, 41 deletions(-)

diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index 115c917..b4ae092 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -46,13 +46,6 @@ static inline struct sync_pt *fence_to_sync_pt(struct fence *fence)
 	return container_of(fence, struct sync_pt, base);
 }
 
-/**
- * sync_timeline_create() - creates a sync object
- * @name:	sync_timeline name
- *
- * Creates a new sync_timeline. Returns the sync_timeline object or NULL in
- * case of error.
- */
 struct sync_timeline *sync_timeline_create(const char *name)
 {
 	struct sync_timeline *obj;
@@ -94,14 +87,6 @@ static void sync_timeline_put(struct sync_timeline *obj)
 	kref_put(&obj->kref, sync_timeline_free);
 }
 
-/**
- * sync_timeline_signal() - signal a status change on a sync_timeline
- * @obj:	sync_timeline to signal
- * @inc:	num to increment on timeline->value
- *
- * A sync implementation should call this any time one of it's fences
- * has signaled or has an error condition.
- */
 static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
 {
 	unsigned long flags;
@@ -122,17 +107,6 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
 	spin_unlock_irqrestore(&obj->child_list_lock, flags);
 }
 
-/**
- * sync_pt_create() - creates a sync pt
- * @parent:	fence's parent sync_timeline
- * @size:	size to allocate for this pt
- * @inc:	value of the fence
- *
- * Creates a new sync_pt as a child of @parent.  @size bytes will be
- * allocated allowing for implementation specific data to be kept after
- * the generic sync_timeline struct. Returns the sync_pt object or
- * NULL in case of error.
- */
 static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size,
 			     unsigned int value)
 {
diff --git a/drivers/staging/android/sync_debug.h b/drivers/staging/android/sync_debug.h
index fab6639..5b82cf8 100644
--- a/drivers/staging/android/sync_debug.h
+++ b/drivers/staging/android/sync_debug.h
@@ -20,15 +20,6 @@
 #include <linux/sync_file.h>
 #include <uapi/linux/sync_file.h>
 
-/**
- * struct sync_timeline - sync object
- * @kref:		reference count on fence.
- * @name:		name of the sync_timeline. Useful for debugging
- * @child_list_head:	list of children sync_pts for this sync_timeline
- * @child_list_lock:	lock protecting @child_list_head and fence.status
- * @active_list_head:	list of active (unsignaled/errored) sync_pts
- * @sync_timeline_list:	membership in global sync_timeline_list
- */
 struct sync_timeline {
 	struct kref		kref;
 	char			name[32];
@@ -51,12 +42,6 @@ static inline struct sync_timeline *fence_parent(struct fence *fence)
 			    child_list_lock);
 }
 
-/**
- * struct sync_pt - sync_pt object
- * @base: base fence object
- * @child_list: sync timeline child's list
- * @active_list: sync timeline active child's list
- */
 struct sync_pt {
 	struct fence base;
 	struct list_head child_list;
-- 
2.5.5

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


#1459428 — Re: [PATCH v2 1/6] staging/android: remove doc from sw_sync

FromPavel Machek <pavel@ucw.cz>
Date2016-08-10 20:30 +0200
SubjectRe: [PATCH v2 1/6] staging/android: remove doc from sw_sync
Message-ID<s4Gqo-b4-113@gated-at.bofh.it>
In reply to#1458255
On Mon 2016-08-08 18:24:17, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> SW_SYNC should never be used by other pieces of the kernel apart from
> sync_debug as it is only a Sync File Validation Framework, so hide any
> info to avoid confuse this with a standard kernel internal API.

> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

NAK.

It is unclear for what the code does, removing the docs is not going to help.

If it should not be used, document that it should not be used.. but not remove
the docs.

> -/**
> - * sync_timeline_signal() - signal a status change on a sync_timeline
> - * @obj:	sync_timeline to signal
> - * @inc:	num to increment on timeline->value
> - *
> - * A sync implementation should call this any time one of it's fences
> - * has signaled or has an error condition.
> - */
>  static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
>  {

And as the functions are static... there's little danger that someone will misuse them.

										Pavel

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web