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 3 patches are clean up and improvements and the rest is preparation to de-stage and then finally the actual de-stage.
Please review,
Gustavo
--- Gustavo Padovan (7): staging/android: move trace/sync.h to sync_trace.h staging/android: remove doc from sw_sync staging/android: display sync_pt name on debugfs staging/android: do not let userspace trigger WARN_ON staging/android: prepare sw_sync files for de-staging dma-buf/sw_sync: de-stage SW_SYNC staging/android: remove sync framework TODO
drivers/dma-buf/Kconfig | 14 +++++++ drivers/dma-buf/Makefile | 1 + drivers/{staging/android => dma-buf}/sw_sync.c | 46 +++------------------- drivers/{staging/android => dma-buf}/sync_debug.c | 7 ++-- drivers/{staging/android => dma-buf}/sync_debug.h | 26 +++++------- .../android/trace/sync.h => dma-buf/sync_trace.h} | 6 +-- drivers/staging/android/Kconfig | 13 ------ drivers/staging/android/Makefile | 1 - drivers/staging/android/TODO | 8 ---- 9 files changed, 38 insertions(+), 84 deletions(-) rename drivers/{staging/android => dma-buf}/sw_sync.c (84%) rename drivers/{staging/android => dma-buf}/sync_debug.c (97%) rename drivers/{staging/android => dma-buf}/sync_debug.h (72%) rename drivers/{staging/android/trace/sync.h => dma-buf/sync_trace.h} (84%)
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 425ebc5..c44f447 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]; @@ -50,12 +41,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;
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
When creating a sync_pt the name received wasn't used anywhere. Now we add it to the sync info debug output to make it easier to indetify the userspace name of that sync pt.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/staging/android/sw_sync.c | 16 ++++------------ drivers/staging/android/sync_debug.c | 5 +++-- drivers/staging/android/sync_debug.h | 9 +++++++++ 3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index b4ae092..ea27512 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -37,15 +37,6 @@ struct sw_sync_create_fence_data { struct sw_sync_create_fence_data) #define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
-static const struct fence_ops timeline_fence_ops; - -static inline struct sync_pt *fence_to_sync_pt(struct fence *fence) -{ - if (fence->ops != &timeline_fence_ops) - return NULL; - return container_of(fence, struct sync_pt, base); -} - struct sync_timeline *sync_timeline_create(const char *name) { struct sync_timeline *obj; @@ -108,7 +99,7 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) }
static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size, - unsigned int value) + unsigned int value, char *name) { unsigned long flags; struct sync_pt *pt; @@ -120,6 +111,7 @@ static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size, if (!pt) return NULL;
+ strlcpy(pt->name, name, sizeof(pt->name)); spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); fence_init(&pt->base, &timeline_fence_ops, &obj->child_list_lock, @@ -191,7 +183,7 @@ static void timeline_fence_timeline_value_str(struct fence *fence, snprintf(str, size, "%d", parent->value); }
-static const struct fence_ops timeline_fence_ops = { +const struct fence_ops timeline_fence_ops = { .get_driver_name = timeline_fence_get_driver_name, .get_timeline_name = timeline_fence_get_timeline_name, .enable_signaling = timeline_fence_enable_signaling, @@ -252,7 +244,7 @@ static long sw_sync_ioctl_create_fence(struct sync_timeline *obj, goto err; }
- pt = sync_pt_create(obj, sizeof(*pt), data.value); + pt = sync_pt_create(obj, sizeof(*pt), data.value, data.name); if (!pt) { err = -ENOMEM; goto err; diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 4c5a855..b732ea3 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -75,13 +75,14 @@ static void sync_print_fence(struct seq_file *s, struct fence *fence, bool show) { int status = 1; struct sync_timeline *parent = fence_parent(fence); + struct sync_pt *pt = fence_to_sync_pt(fence);
if (fence_is_signaled_locked(fence)) status = fence->status;
- seq_printf(s, " %s%sfence %s", + seq_printf(s, " %s%sfence %s %s", show ? parent->name : "", - show ? "_" : "", + show ? "_" : "", pt->name, sync_status_str(status));
if (status <= 0) { diff --git a/drivers/staging/android/sync_debug.h b/drivers/staging/android/sync_debug.h index c44f447..c14587c 100644 --- a/drivers/staging/android/sync_debug.h +++ b/drivers/staging/android/sync_debug.h @@ -43,10 +43,19 @@ static inline struct sync_timeline *fence_parent(struct fence *fence)
struct sync_pt { struct fence base; + char name[32]; struct list_head child_list; struct list_head active_list; };
+extern const struct fence_ops timeline_fence_ops; +static inline struct sync_pt *fence_to_sync_pt(struct fence *fence) +{ + if (fence->ops != &timeline_fence_ops) + return NULL; + return container_of(fence, struct sync_pt, base); +} + #ifdef CONFIG_SW_SYNC
extern const struct file_operations sw_sync_debugfs_fops;
Op 20-06-16 om 17:53 schreef Gustavo Padovan:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
When creating a sync_pt the name received wasn't used anywhere. Now we add it to the sync info debug output to make it easier to indetify the userspace name of that sync pt.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/staging/android/sw_sync.c | 16 ++++------------ drivers/staging/android/sync_debug.c | 5 +++-- drivers/staging/android/sync_debug.h | 9 +++++++++ 3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index b4ae092..ea27512 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -37,15 +37,6 @@ struct sw_sync_create_fence_data { struct sw_sync_create_fence_data) #define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
-static const struct fence_ops timeline_fence_ops;
-static inline struct sync_pt *fence_to_sync_pt(struct fence *fence) -{
- if (fence->ops != &timeline_fence_ops)
return NULL;
- return container_of(fence, struct sync_pt, base);
-}
struct sync_timeline *sync_timeline_create(const char *name) { struct sync_timeline *obj; @@ -108,7 +99,7 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) }
static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size,
unsigned int value)
unsigned int value, char *name)
{ unsigned long flags; struct sync_pt *pt; @@ -120,6 +111,7 @@ static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size, if (!pt) return NULL;
- strlcpy(pt->name, name, sizeof(pt->name)); spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); fence_init(&pt->base, &timeline_fence_ops, &obj->child_list_lock,
@@ -191,7 +183,7 @@ static void timeline_fence_timeline_value_str(struct fence *fence, snprintf(str, size, "%d", parent->value); }
-static const struct fence_ops timeline_fence_ops = { +const struct fence_ops timeline_fence_ops = { .get_driver_name = timeline_fence_get_driver_name, .get_timeline_name = timeline_fence_get_timeline_name, .enable_signaling = timeline_fence_enable_signaling, @@ -252,7 +244,7 @@ static long sw_sync_ioctl_create_fence(struct sync_timeline *obj, goto err; }
- pt = sync_pt_create(obj, sizeof(*pt), data.value);
- pt = sync_pt_create(obj, sizeof(*pt), data.value, data.name); if (!pt) { err = -ENOMEM; goto err;
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 4c5a855..b732ea3 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -75,13 +75,14 @@ static void sync_print_fence(struct seq_file *s, struct fence *fence, bool show) { int status = 1; struct sync_timeline *parent = fence_parent(fence);
struct sync_pt *pt = fence_to_sync_pt(fence);
if (fence_is_signaled_locked(fence)) status = fence->status;
- seq_printf(s, " %s%sfence %s",
- seq_printf(s, " %s%sfence %s %s", show ? parent->name : "",
show ? "_" : "",
sync_status_str(status));show ? "_" : "", pt->name,
NAK, A fence in sync_print_fence can be of any type. If you want to print the name, use the fence_value_str callback.
~Maarten
2016-08-08 Maarten Lankhorst maarten.lankhorst@linux.intel.com:
Op 20-06-16 om 17:53 schreef Gustavo Padovan:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
When creating a sync_pt the name received wasn't used anywhere. Now we add it to the sync info debug output to make it easier to indetify the userspace name of that sync pt.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/staging/android/sw_sync.c | 16 ++++------------ drivers/staging/android/sync_debug.c | 5 +++-- drivers/staging/android/sync_debug.h | 9 +++++++++ 3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index b4ae092..ea27512 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -37,15 +37,6 @@ struct sw_sync_create_fence_data { struct sw_sync_create_fence_data) #define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
-static const struct fence_ops timeline_fence_ops;
-static inline struct sync_pt *fence_to_sync_pt(struct fence *fence) -{
- if (fence->ops != &timeline_fence_ops)
return NULL;
- return container_of(fence, struct sync_pt, base);
-}
struct sync_timeline *sync_timeline_create(const char *name) { struct sync_timeline *obj; @@ -108,7 +99,7 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) }
static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size,
unsigned int value)
unsigned int value, char *name)
{ unsigned long flags; struct sync_pt *pt; @@ -120,6 +111,7 @@ static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size, if (!pt) return NULL;
- strlcpy(pt->name, name, sizeof(pt->name)); spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); fence_init(&pt->base, &timeline_fence_ops, &obj->child_list_lock,
@@ -191,7 +183,7 @@ static void timeline_fence_timeline_value_str(struct fence *fence, snprintf(str, size, "%d", parent->value); }
-static const struct fence_ops timeline_fence_ops = { +const struct fence_ops timeline_fence_ops = { .get_driver_name = timeline_fence_get_driver_name, .get_timeline_name = timeline_fence_get_timeline_name, .enable_signaling = timeline_fence_enable_signaling, @@ -252,7 +244,7 @@ static long sw_sync_ioctl_create_fence(struct sync_timeline *obj, goto err; }
- pt = sync_pt_create(obj, sizeof(*pt), data.value);
- pt = sync_pt_create(obj, sizeof(*pt), data.value, data.name); if (!pt) { err = -ENOMEM; goto err;
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 4c5a855..b732ea3 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -75,13 +75,14 @@ static void sync_print_fence(struct seq_file *s, struct fence *fence, bool show) { int status = 1; struct sync_timeline *parent = fence_parent(fence);
struct sync_pt *pt = fence_to_sync_pt(fence);
if (fence_is_signaled_locked(fence)) status = fence->status;
- seq_printf(s, " %s%sfence %s",
- seq_printf(s, " %s%sfence %s %s", show ? parent->name : "",
show ? "_" : "",
sync_status_str(status));show ? "_" : "", pt->name,
NAK, A fence in sync_print_fence can be of any type. If you want to print the name, use the fence_value_str callback.
Indeed. But fence_value_str doesn't return the sync_pt name, but the seqno of that fence. I'll keep this change out for the de-staging and then try to come with something that works better.
Gustavo
Op 08-08-16 om 21:59 schreef Gustavo Padovan:
2016-08-08 Maarten Lankhorst maarten.lankhorst@linux.intel.com:
Op 20-06-16 om 17:53 schreef Gustavo Padovan:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
When creating a sync_pt the name received wasn't used anywhere. Now we add it to the sync info debug output to make it easier to indetify the userspace name of that sync pt.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/staging/android/sw_sync.c | 16 ++++------------ drivers/staging/android/sync_debug.c | 5 +++-- drivers/staging/android/sync_debug.h | 9 +++++++++ 3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index b4ae092..ea27512 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -37,15 +37,6 @@ struct sw_sync_create_fence_data { struct sw_sync_create_fence_data) #define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
-static const struct fence_ops timeline_fence_ops;
-static inline struct sync_pt *fence_to_sync_pt(struct fence *fence) -{
- if (fence->ops != &timeline_fence_ops)
return NULL;
- return container_of(fence, struct sync_pt, base);
-}
struct sync_timeline *sync_timeline_create(const char *name) { struct sync_timeline *obj; @@ -108,7 +99,7 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) }
static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size,
unsigned int value)
unsigned int value, char *name)
{ unsigned long flags; struct sync_pt *pt; @@ -120,6 +111,7 @@ static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size, if (!pt) return NULL;
- strlcpy(pt->name, name, sizeof(pt->name)); spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); fence_init(&pt->base, &timeline_fence_ops, &obj->child_list_lock,
@@ -191,7 +183,7 @@ static void timeline_fence_timeline_value_str(struct fence *fence, snprintf(str, size, "%d", parent->value); }
-static const struct fence_ops timeline_fence_ops = { +const struct fence_ops timeline_fence_ops = { .get_driver_name = timeline_fence_get_driver_name, .get_timeline_name = timeline_fence_get_timeline_name, .enable_signaling = timeline_fence_enable_signaling, @@ -252,7 +244,7 @@ static long sw_sync_ioctl_create_fence(struct sync_timeline *obj, goto err; }
- pt = sync_pt_create(obj, sizeof(*pt), data.value);
- pt = sync_pt_create(obj, sizeof(*pt), data.value, data.name); if (!pt) { err = -ENOMEM; goto err;
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 4c5a855..b732ea3 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -75,13 +75,14 @@ static void sync_print_fence(struct seq_file *s, struct fence *fence, bool show) { int status = 1; struct sync_timeline *parent = fence_parent(fence);
struct sync_pt *pt = fence_to_sync_pt(fence);
if (fence_is_signaled_locked(fence)) status = fence->status;
- seq_printf(s, " %s%sfence %s",
- seq_printf(s, " %s%sfence %s %s", show ? parent->name : "",
show ? "_" : "",
sync_status_str(status));show ? "_" : "", pt->name,
NAK, A fence in sync_print_fence can be of any type. If you want to print the name, use the fence_value_str callback.
Indeed. But fence_value_str doesn't return the sync_pt name, but the seqno of that fence. I'll keep this change out for the de-staging and then try to come with something that works better.
Gustavo
This will probably cause a kernel panic if you keep it like it is and you look at debugfs for non sync_pt fences, it should definitely be fixed before destaging. Ignoring sync_pt name would be better than crashing.
~Maarten
2016-08-09 Maarten Lankhorst maarten.lankhorst@linux.intel.com:
Op 08-08-16 om 21:59 schreef Gustavo Padovan:
2016-08-08 Maarten Lankhorst maarten.lankhorst@linux.intel.com:
Op 20-06-16 om 17:53 schreef Gustavo Padovan:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
When creating a sync_pt the name received wasn't used anywhere. Now we add it to the sync info debug output to make it easier to indetify the userspace name of that sync pt.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/staging/android/sw_sync.c | 16 ++++------------ drivers/staging/android/sync_debug.c | 5 +++-- drivers/staging/android/sync_debug.h | 9 +++++++++ 3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index b4ae092..ea27512 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -37,15 +37,6 @@ struct sw_sync_create_fence_data { struct sw_sync_create_fence_data) #define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
-static const struct fence_ops timeline_fence_ops;
-static inline struct sync_pt *fence_to_sync_pt(struct fence *fence) -{
- if (fence->ops != &timeline_fence_ops)
return NULL;
- return container_of(fence, struct sync_pt, base);
-}
struct sync_timeline *sync_timeline_create(const char *name) { struct sync_timeline *obj; @@ -108,7 +99,7 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc) }
static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size,
unsigned int value)
unsigned int value, char *name)
{ unsigned long flags; struct sync_pt *pt; @@ -120,6 +111,7 @@ static struct sync_pt *sync_pt_create(struct sync_timeline *obj, int size, if (!pt) return NULL;
- strlcpy(pt->name, name, sizeof(pt->name)); spin_lock_irqsave(&obj->child_list_lock, flags); sync_timeline_get(obj); fence_init(&pt->base, &timeline_fence_ops, &obj->child_list_lock,
@@ -191,7 +183,7 @@ static void timeline_fence_timeline_value_str(struct fence *fence, snprintf(str, size, "%d", parent->value); }
-static const struct fence_ops timeline_fence_ops = { +const struct fence_ops timeline_fence_ops = { .get_driver_name = timeline_fence_get_driver_name, .get_timeline_name = timeline_fence_get_timeline_name, .enable_signaling = timeline_fence_enable_signaling, @@ -252,7 +244,7 @@ static long sw_sync_ioctl_create_fence(struct sync_timeline *obj, goto err; }
- pt = sync_pt_create(obj, sizeof(*pt), data.value);
- pt = sync_pt_create(obj, sizeof(*pt), data.value, data.name); if (!pt) { err = -ENOMEM; goto err;
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index 4c5a855..b732ea3 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -75,13 +75,14 @@ static void sync_print_fence(struct seq_file *s, struct fence *fence, bool show) { int status = 1; struct sync_timeline *parent = fence_parent(fence);
struct sync_pt *pt = fence_to_sync_pt(fence);
if (fence_is_signaled_locked(fence)) status = fence->status;
- seq_printf(s, " %s%sfence %s",
- seq_printf(s, " %s%sfence %s %s", show ? parent->name : "",
show ? "_" : "",
sync_status_str(status));show ? "_" : "", pt->name,
NAK, A fence in sync_print_fence can be of any type. If you want to print the name, use the fence_value_str callback.
Indeed. But fence_value_str doesn't return the sync_pt name, but the seqno of that fence. I'll keep this change out for the de-staging and then try to come with something that works better.
Gustavo
This will probably cause a kernel panic if you keep it like it is and you look at debugfs for non sync_pt fences, it should definitely be fixed before destaging. Ignoring sync_pt name would be better than crashing.
No, I meant ignoring sync_pt name for now. I've already sent v2 without it.
Gustavo
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 ea27512..66837ca 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -142,7 +142,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);
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/{trace/sync.h => sync_trace.h} | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) rename drivers/staging/android/{trace/sync.h => sync_trace.h} (84%)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 66837ca..25196f5 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/trace/sync.h b/drivers/staging/android/sync_trace.h similarity index 84% rename from drivers/staging/android/trace/sync.h rename to drivers/staging/android/sync_trace.h index 6b5ce96..ea485f7 100644 --- a/drivers/staging/android/trace/sync.h +++ b/drivers/staging/android/sync_trace.h @@ -1,11 +1,11 @@ #undef TRACE_SYSTEM -#define TRACE_INCLUDE_PATH ../../drivers/staging/android/trace -#define TRACE_SYSTEM sync +#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 "sync_debug.h" #include <linux/tracepoint.h>
TRACE_EVENT(sync_timeline,
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Fix paths in the comments.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/staging/android/sync_debug.c | 2 +- drivers/staging/android/sync_debug.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index b732ea3..5f0d41e 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -1,5 +1,5 @@ /* - * drivers/base/sync.c + * drivers/dma-buf/sync_debug.c * * Copyright (C) 2012 Google, Inc. * diff --git a/drivers/staging/android/sync_debug.h b/drivers/staging/android/sync_debug.h index c14587c..e359047 100644 --- a/drivers/staging/android/sync_debug.h +++ b/drivers/staging/android/sync_debug.h @@ -1,5 +1,5 @@ /* - * include/linux/sync.h + * drivers/dma-buf/sync_debug.h * * Copyright (C) 2012 Google, Inc. *
On Mon, 2016-06-20 at 12:53 -0300, Gustavo Padovan wrote:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Fix paths in the comments.
Why is it useful to have the path or filename embedded in the file at all?
2016-06-20 Joe Perches joe@perches.com:
On Mon, 2016-06-20 at 12:53 -0300, Gustavo Padovan wrote:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Fix paths in the comments.
Why is it useful to have the path or filename embedded in the file at all?
I just kept it as is. Thinking about this now I don't see this as useful, I'll send v2 removing it.
Gustavo
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
remove file paths in the comments and add short description about each file.
v2: remove file paths instead of just change them.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/staging/android/sw_sync.c | 2 +- drivers/staging/android/sync_debug.c | 2 +- drivers/staging/android/sync_debug.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 25196f5..a355e9b 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -1,5 +1,5 @@ /* - * drivers/dma-buf/sw_sync.c + * Sync File validation framework * * Copyright (C) 2012 Google, Inc. * diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c index b732ea3..03bcc6f 100644 --- a/drivers/staging/android/sync_debug.c +++ b/drivers/staging/android/sync_debug.c @@ -1,5 +1,5 @@ /* - * drivers/base/sync.c + * Sync File validation framework and debug information * * Copyright (C) 2012 Google, Inc. * diff --git a/drivers/staging/android/sync_debug.h b/drivers/staging/android/sync_debug.h index c14587c..63cfcff 100644 --- a/drivers/staging/android/sync_debug.h +++ b/drivers/staging/android/sync_debug.h @@ -1,5 +1,5 @@ /* - * include/linux/sync.h + * Sync File validation framework * * Copyright (C) 2012 Google, Inc. *
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
SW_SYNC allows to run tests on the sync_file framework via debugfs on
<debugfs>/sync/sw_sync
Opening and closing the file triggers creation and release of a sync timeline. To create fences on this timeline the SW_SYNC_IOC_CREATE_FENCE ioctl should be used. To increment the timeline value use SW_SYNC_IOC_INC.
Also it exports Sync information on
<debugfs>/sync/info
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/dma-buf/Kconfig | 14 ++++++++++++++ drivers/dma-buf/Makefile | 1 + drivers/{staging/android => dma-buf}/sw_sync.c | 0 drivers/{staging/android => dma-buf}/sync_debug.c | 0 drivers/{staging/android => dma-buf}/sync_debug.h | 0 drivers/{staging/android => dma-buf}/sync_trace.h | 2 +- drivers/staging/android/Kconfig | 13 ------------- drivers/staging/android/Makefile | 1 - 8 files changed, 16 insertions(+), 15 deletions(-) rename drivers/{staging/android => dma-buf}/sw_sync.c (100%) rename drivers/{staging/android => dma-buf}/sync_debug.c (100%) rename drivers/{staging/android => dma-buf}/sync_debug.h (100%) rename drivers/{staging/android => dma-buf}/sync_trace.h (92%)
diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig index 9824bc4..7227022 100644 --- a/drivers/dma-buf/Kconfig +++ b/drivers/dma-buf/Kconfig @@ -8,4 +8,18 @@ config SYNC_FILE ---help--- This option enables the fence framework synchronization to export sync_files to userspace that can represent one or more fences. + +config SW_SYNC + bool "Sync File Validation Framework" + default n + depends on SYNC_FILE + depends on DEBUG_FS + ---help--- + A sync object driver that uses a 32bit counter to coordinate + synchronization. Useful when there is no hardware primitive backing + the synchronization. + + WARNING: improper use of this can result in deadlocking kernel + drivers from userspace. Intended for test and debug only. + endmenu diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 4a424ec..d2d02fc 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,2 +1,3 @@ obj-y := dma-buf.o fence.o reservation.o seqno-fence.o obj-$(CONFIG_SYNC_FILE) += sync_file.o +obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o diff --git a/drivers/staging/android/sw_sync.c b/drivers/dma-buf/sw_sync.c similarity index 100% rename from drivers/staging/android/sw_sync.c rename to drivers/dma-buf/sw_sync.c diff --git a/drivers/staging/android/sync_debug.c b/drivers/dma-buf/sync_debug.c similarity index 100% rename from drivers/staging/android/sync_debug.c rename to drivers/dma-buf/sync_debug.c diff --git a/drivers/staging/android/sync_debug.h b/drivers/dma-buf/sync_debug.h similarity index 100% rename from drivers/staging/android/sync_debug.h rename to drivers/dma-buf/sync_debug.h diff --git a/drivers/staging/android/sync_trace.h b/drivers/dma-buf/sync_trace.h similarity index 92% rename from drivers/staging/android/sync_trace.h rename to drivers/dma-buf/sync_trace.h index ea485f7..d13d59f 100644 --- a/drivers/staging/android/sync_trace.h +++ b/drivers/dma-buf/sync_trace.h @@ -1,5 +1,5 @@ #undef TRACE_SYSTEM -#define TRACE_INCLUDE_PATH ../../drivers/staging/android +#define TRACE_INCLUDE_PATH ../../drivers/dma-buf #define TRACE_SYSTEM sync_trace
#if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig index 06e41d2..6c00d6f 100644 --- a/drivers/staging/android/Kconfig +++ b/drivers/staging/android/Kconfig @@ -24,19 +24,6 @@ config ANDROID_LOW_MEMORY_KILLER scripts (/init.rc), and it defines priority values with minimum free memory size for each priority.
-config SW_SYNC - bool "Software synchronization framework" - default n - depends on SYNC_FILE - depends on DEBUG_FS - ---help--- - A sync object driver that uses a 32bit counter to coordinate - synchronization. Useful when there is no hardware primitive backing - the synchronization. - - WARNING: improper use of this can result in deadlocking kernel - drivers from userspace. Intended for test and debug only. - source "drivers/staging/android/ion/Kconfig"
endif # if ANDROID diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index 7ca61b7..7ed1be7 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -4,4 +4,3 @@ obj-y += ion/
obj-$(CONFIG_ASHMEM) += ashmem.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER) += lowmemorykiller.o -obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Sync Framework was de-staged to drivers/dma-buf/, so remove it entries in the TODO file.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/staging/android/TODO | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO index 64d8c87..8f3ac37 100644 --- a/drivers/staging/android/TODO +++ b/drivers/staging/android/TODO @@ -25,13 +25,5 @@ ion/ exposes existing cma regions and doesn't reserve unecessarily memory when booting a system which doesn't use ion.
-sync framework: - - remove CONFIG_SW_SYNC_USER, it is used only for testing/debugging and - should not be upstreamed. - - port CONFIG_SW_SYNC_USER tests interfaces to use debugfs somehow - - port libsync tests to kselftest - - clean up and ABI check for security issues - - move it to drivers/base/dma-buf - Please send patches to Greg Kroah-Hartman greg@kroah.com and Cc: Arve Hjønnevåg arve@android.com and Riley Andrews riandrews@android.com
Hi Gustavo,
On 20 June 2016 at 16:53, Gustavo Padovan gustavo@padovan.org wrote:
- port libsync tests to kselftest
I believe the tests haven't landed yet right, so this should stay right ?
-Emil
2016-06-23 Emil Velikov emil.l.velikov@gmail.com:
Hi Gustavo,
On 20 June 2016 at 16:53, Gustavo Padovan gustavo@padovan.org wrote:
- port libsync tests to kselftest
I believe the tests haven't landed yet right, so this should stay right ?
Yes, you are right. That part is still missing in upstream.
Gustavo
Hi!
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Hi Greg,
This is the last step in the Sync Framwork de-stage task. It
Typo: "fram_e_work"
de-stage the SW_SYNC validation framework and the sync_debug info debugfs file.
The first 3 patches are clean up and improvements and the rest is preparation to de-stage and then finally the actual de-stage.
Could we get some kind of description what the sync framework does? There are no useful comments in /sw_sync.c. There's no Documentation/ files. I don't know what this is supposed to do...
Pavel
Please review,
Gustavo
Gustavo Padovan (7): staging/android: move trace/sync.h to sync_trace.h staging/android: remove doc from sw_sync staging/android: display sync_pt name on debugfs staging/android: do not let userspace trigger WARN_ON staging/android: prepare sw_sync files for de-staging dma-buf/sw_sync: de-stage SW_SYNC staging/android: remove sync framework TODO
drivers/dma-buf/Kconfig | 14 +++++++ drivers/dma-buf/Makefile | 1 + drivers/{staging/android => dma-buf}/sw_sync.c | 46 +++------------------- drivers/{staging/android => dma-buf}/sync_debug.c | 7 ++-- drivers/{staging/android => dma-buf}/sync_debug.h | 26 +++++------- .../android/trace/sync.h => dma-buf/sync_trace.h} | 6 +-- drivers/staging/android/Kconfig | 13 ------ drivers/staging/android/Makefile | 1 - drivers/staging/android/TODO | 8 ---- 9 files changed, 38 insertions(+), 84 deletions(-) rename drivers/{staging/android => dma-buf}/sw_sync.c (84%) rename drivers/{staging/android => dma-buf}/sync_debug.c (97%) rename drivers/{staging/android => dma-buf}/sync_debug.h (72%) rename drivers/{staging/android/trace/sync.h => dma-buf/sync_trace.h} (84%)
Hi Greg,
Any comment on this?
Thanks,
Gustavo
2016-06-20 Gustavo Padovan gustavo@padovan.org:
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 3 patches are clean up and improvements and the rest is preparation to de-stage and then finally the actual de-stage.
Please review,
Gustavo
Gustavo Padovan (7): staging/android: move trace/sync.h to sync_trace.h staging/android: remove doc from sw_sync staging/android: display sync_pt name on debugfs staging/android: do not let userspace trigger WARN_ON staging/android: prepare sw_sync files for de-staging dma-buf/sw_sync: de-stage SW_SYNC staging/android: remove sync framework TODO
drivers/dma-buf/Kconfig | 14 +++++++ drivers/dma-buf/Makefile | 1 + drivers/{staging/android => dma-buf}/sw_sync.c | 46 +++------------------- drivers/{staging/android => dma-buf}/sync_debug.c | 7 ++-- drivers/{staging/android => dma-buf}/sync_debug.h | 26 +++++------- .../android/trace/sync.h => dma-buf/sync_trace.h} | 6 +-- drivers/staging/android/Kconfig | 13 ------ drivers/staging/android/Makefile | 1 - drivers/staging/android/TODO | 8 ---- 9 files changed, 38 insertions(+), 84 deletions(-) rename drivers/{staging/android => dma-buf}/sw_sync.c (84%) rename drivers/{staging/android => dma-buf}/sync_debug.c (97%) rename drivers/{staging/android => dma-buf}/sync_debug.h (72%) rename drivers/{staging/android/trace/sync.h => dma-buf/sync_trace.h} (84%)
-- 2.5.5
Hi,
Do you think there is time to get this in for 4.8?
Thanks.
2016-06-20 Gustavo Padovan gustavo@padovan.org:
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 3 patches are clean up and improvements and the rest is preparation to de-stage and then finally the actual de-stage.
Please review,
Gustavo
Gustavo Padovan (7): staging/android: move trace/sync.h to sync_trace.h staging/android: remove doc from sw_sync staging/android: display sync_pt name on debugfs staging/android: do not let userspace trigger WARN_ON staging/android: prepare sw_sync files for de-staging dma-buf/sw_sync: de-stage SW_SYNC staging/android: remove sync framework TODO
drivers/dma-buf/Kconfig | 14 +++++++ drivers/dma-buf/Makefile | 1 + drivers/{staging/android => dma-buf}/sw_sync.c | 46 +++------------------- drivers/{staging/android => dma-buf}/sync_debug.c | 7 ++-- drivers/{staging/android => dma-buf}/sync_debug.h | 26 +++++------- .../android/trace/sync.h => dma-buf/sync_trace.h} | 6 +-- drivers/staging/android/Kconfig | 13 ------ drivers/staging/android/Makefile | 1 - drivers/staging/android/TODO | 8 ---- 9 files changed, 38 insertions(+), 84 deletions(-) rename drivers/{staging/android => dma-buf}/sw_sync.c (84%) rename drivers/{staging/android => dma-buf}/sync_debug.c (97%) rename drivers/{staging/android => dma-buf}/sync_debug.h (72%) rename drivers/{staging/android/trace/sync.h => dma-buf/sync_trace.h} (84%)
-- 2.5.5
Gustavo
On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
Hi,
Do you think there is time to get this in for 4.8?
No, it was too late on my end, due to travel and vacation, sorry. I'll queue it up for 4.9-rc1.
Could we get some documentation what this does? Is it visilble to userspace?
Pavel
2016-08-07 Pavel Machek pavel@ucw.cz:
On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
Hi,
Do you think there is time to get this in for 4.8?
No, it was too late on my end, due to travel and vacation, sorry. I'll queue it up for 4.9-rc1.
Could we get some documentation what this does? Is it visilble to userspace?
This interface is only intended for testing and validation, there are ioctls on the debugfs file that can be accessed by userspace but there isn't any exported kernel header with this info. The tester should know and add a internal header to be able to access it. We want to prevent people from misusing this feature by not advertising it nor providing documentation.
There will be, though, kselftest for this interface, that I send out once SW_SYNC is de-staged.
On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote:
2016-08-07 Pavel Machek pavel@ucw.cz:
On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
Hi,
Do you think there is time to get this in for 4.8?
No, it was too late on my end, due to travel and vacation, sorry. I'll queue it up for 4.9-rc1.
Could we get some documentation what this does? Is it visilble to userspace?
This interface is only intended for testing and validation, there are ioctls on the debugfs file that can be accessed by userspace but there isn't any exported kernel header with this info. The tester should know and add a internal header to be able to access it. We want to prevent people from misusing this feature by not advertising it nor providing documentation.
You are playing dangerous game here. debugfs is not normally considered stable, but otoh... ioctls on debugfs?
Anyway, please provide some documentation. Kernel hackers need to know what this does.
Pavel
2016-07-24 Pavel Machek pavel@ucw.cz:
On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote:
2016-08-07 Pavel Machek pavel@ucw.cz:
On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
Hi,
Do you think there is time to get this in for 4.8?
No, it was too late on my end, due to travel and vacation, sorry. I'll queue it up for 4.9-rc1.
Could we get some documentation what this does? Is it visilble to userspace?
This interface is only intended for testing and validation, there are ioctls on the debugfs file that can be accessed by userspace but there isn't any exported kernel header with this info. The tester should know and add a internal header to be able to access it. We want to prevent people from misusing this feature by not advertising it nor providing documentation.
You are playing dangerous game here. debugfs is not normally considered stable, but otoh... ioctls on debugfs?
Anyway, please provide some documentation. Kernel hackers need to know what this does.
Okay, where do you think is the best place? Would documentation inside the .c file suffice for you?
On Sun, Jul 24, 2016 at 05:00:31PM +0200, Pavel Machek wrote:
On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote:
2016-08-07 Pavel Machek pavel@ucw.cz:
On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
Hi,
Do you think there is time to get this in for 4.8?
No, it was too late on my end, due to travel and vacation, sorry. I'll queue it up for 4.9-rc1.
Could we get some documentation what this does? Is it visilble to userspace?
This interface is only intended for testing and validation, there are ioctls on the debugfs file that can be accessed by userspace but there isn't any exported kernel header with this info. The tester should know and add a internal header to be able to access it. We want to prevent people from misusing this feature by not advertising it nor providing documentation.
You are playing dangerous game here. debugfs is not normally considered stable, but otoh... ioctls on debugfs?
It's not considered stable. The idea is that we also add the existing testcases to kselftest. It's purely a bit of interface to be able to drive run the test logic for real fences. What it really tests is the fence interface (which is public in the uapi headers and all that), but to be able to do that we need some (hw-independent way) to expose fences, which this provides.
Long term we might even do this as a proper interface (with some restrictions to make it safe and avoid userspace pulling the kernel over the table). And then rip out sw_sync entirely.
Imo there's no need at all for docs for this. -Daniel
On Tue 2016-08-09 08:04:54, Daniel Vetter wrote:
On Sun, Jul 24, 2016 at 05:00:31PM +0200, Pavel Machek wrote:
On Mon 2016-08-08 16:08:12, Gustavo Padovan wrote:
2016-08-07 Pavel Machek pavel@ucw.cz:
On Sun 2016-07-24 15:21:11, Greg Kroah-Hartman wrote:
On Mon, Jul 18, 2016 at 04:12:45PM -0300, Gustavo Padovan wrote:
Hi,
Do you think there is time to get this in for 4.8?
No, it was too late on my end, due to travel and vacation, sorry. I'll queue it up for 4.9-rc1.
Could we get some documentation what this does? Is it visilble to userspace?
This interface is only intended for testing and validation, there are ioctls on the debugfs file that can be accessed by userspace but there isn't any exported kernel header with this info. The tester should know and add a internal header to be able to access it. We want to prevent people from misusing this feature by not advertising it nor providing documentation.
You are playing dangerous game here. debugfs is not normally considered stable, but otoh... ioctls on debugfs?
It's not considered stable. The idea is that we also add the existing testcases to kselftest. It's purely a bit of interface to be able to drive run the test logic for real fences. What it really tests is the fence interface (which is public in the uapi headers and all that), but to be able to do that we need some (hw-independent way) to expose fences, which this provides.
Long term we might even do this as a proper interface (with some restrictions to make it safe and avoid userspace pulling the kernel over the table). And then rip out sw_sync entirely.
Imo there's no need at all for docs for this.
There's full directory of files, with absolutely zero comments/documentation. They are quite hard to understand. Plus it has userland interface.
IMO comment should be added, explaining what it is testing, that interface is not considered stable, and where the test lives.
I know what "fence" is in the cpu sense (mfence and friends), but I'm not sure what "real fence" is in this context.
Best regards, Pavel
dri-devel@lists.freedesktop.org