Re: [Intel-gfx] [PATCH 01/13] staging/android/sync: Support sync points created from dma-fences

[Jesse Barnes]

On 12/11/2015 05:11 AM, john.c.harri...@intel.com wrote:
> From: Maarten Lankhorst 
> 
> Debug output assumes all sync points are built on top of Android sync points
> and when we start creating them from dma-fences will NULL ptr deref unless
> taught about this.
> 
> v4: Corrected patch ownership.
> 
> Signed-off-by: Maarten Lankhorst 
> Signed-off-by: Tvrtko Ursulin 
> Cc: Maarten Lankhorst 
> Cc: de...@driverdev.osuosl.org
> Cc: Riley Andrews 
> Cc: Greg Kroah-Hartman 
> Cc: Arve Hjønnevåg 
> ---
>  drivers/staging/android/sync_debug.c | 42 
> +++-
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/staging/android/sync_debug.c 
> b/drivers/staging/android/sync_debug.c
> index 91ed2c4..f45d13c 100644
> --- a/drivers/staging/android/sync_debug.c
> +++ b/drivers/staging/android/sync_debug.c
> @@ -82,36 +82,42 @@ static const char *sync_status_str(int status)
>   return "error";
>  }
>  
> -static void sync_print_pt(struct seq_file *s, struct sync_pt *pt, bool fence)
> +static void sync_print_pt(struct seq_file *s, struct fence *pt, bool fence)
>  {
>   int status = 1;
> - struct sync_timeline *parent = sync_pt_parent(pt);
>  
> - if (fence_is_signaled_locked(>base))
> - status = pt->base.status;
> + if (fence_is_signaled_locked(pt))
> + status = pt->status;
>  
>   seq_printf(s, "  %s%spt %s",
> -fence ? parent->name : "",
> +fence && pt->ops->get_timeline_name ?
> +pt->ops->get_timeline_name(pt) : "",
>  fence ? "_" : "",
>  sync_status_str(status));
>  
>   if (status <= 0) {
>   struct timespec64 ts64 =
> - ktime_to_timespec64(pt->base.timestamp);
> + ktime_to_timespec64(pt->timestamp);
>  
>   seq_printf(s, "@%lld.%09ld", (s64)ts64.tv_sec, ts64.tv_nsec);
>   }
>  
> - if (parent->ops->timeline_value_str &&
> - parent->ops->pt_value_str) {
> + if ((!fence || pt->ops->timeline_value_str) &&
> + pt->ops->fence_value_str) {
>   char value[64];
> + bool success;
>  
> - parent->ops->pt_value_str(pt, value, sizeof(value));
> - seq_printf(s, ": %s", value);
> - if (fence) {
> - parent->ops->timeline_value_str(parent, value,
> - sizeof(value));
> - seq_printf(s, " / %s", value);
> + pt->ops->fence_value_str(pt, value, sizeof(value));
> + success = strlen(value);
> +
> + if (success)
> + seq_printf(s, ": %s", value);
> +
> + if (success && fence) {
> + pt->ops->timeline_value_str(pt, value, sizeof(value));
> +
> + if (strlen(value))
> + seq_printf(s, " / %s", value);
>   }
>   }
>  
> @@ -138,7 +144,7 @@ static void sync_print_obj(struct seq_file *s, struct 
> sync_timeline *obj)
>   list_for_each(pos, >child_list_head) {
>   struct sync_pt *pt =
>   container_of(pos, struct sync_pt, child_list);
> - sync_print_pt(s, pt, false);
> + sync_print_pt(s, >base, false);
>   }
>   spin_unlock_irqrestore(>child_list_lock, flags);
>  }
> @@ -153,11 +159,7 @@ static void sync_print_fence(struct seq_file *s, struct 
> sync_fence *fence)
>  sync_status_str(atomic_read(>status)));
>  
>   for (i = 0; i < fence->num_fences; ++i) {
> - struct sync_pt *pt =
> - container_of(fence->cbs[i].sync_pt,
> -  struct sync_pt, base);
> -
> - sync_print_pt(s, pt, true);
> + sync_print_pt(s, fence->cbs[i].sync_pt, true);
>   }
>  
>   spin_lock_irqsave(>wq.lock, flags);
> 

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 01/13] staging/android/sync: Support sync points created from dma-fences

[John . C . Harrison]

From: Maarten Lankhorst 

Debug output assumes all sync points are built on top of Android sync points
and when we start creating them from dma-fences will NULL ptr deref unless
taught about this.

v4: Corrected patch ownership.

Signed-off-by: Maarten Lankhorst 
Signed-off-by: Tvrtko Ursulin 
Cc: Maarten Lankhorst 
Cc: de...@driverdev.osuosl.org
Cc: Riley Andrews 
Cc: Greg Kroah-Hartman 
Cc: Arve Hjønnevåg 
---
 drivers/staging/android/sync_debug.c | 42 +++-
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/android/sync_debug.c 
b/drivers/staging/android/sync_debug.c
index 91ed2c4..f45d13c 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -82,36 +82,42 @@ static const char *sync_status_str(int status)
return "error";
 }
 
-static void sync_print_pt(struct seq_file *s, struct sync_pt *pt, bool fence)
+static void sync_print_pt(struct seq_file *s, struct fence *pt, bool fence)
 {
int status = 1;
-   struct sync_timeline *parent = sync_pt_parent(pt);
 
-   if (fence_is_signaled_locked(>base))
-   status = pt->base.status;
+   if (fence_is_signaled_locked(pt))
+   status = pt->status;
 
seq_printf(s, "  %s%spt %s",
-  fence ? parent->name : "",
+  fence && pt->ops->get_timeline_name ?
+  pt->ops->get_timeline_name(pt) : "",
   fence ? "_" : "",
   sync_status_str(status));
 
if (status <= 0) {
struct timespec64 ts64 =
-   ktime_to_timespec64(pt->base.timestamp);
+   ktime_to_timespec64(pt->timestamp);
 
seq_printf(s, "@%lld.%09ld", (s64)ts64.tv_sec, ts64.tv_nsec);
}
 
-   if (parent->ops->timeline_value_str &&
-   parent->ops->pt_value_str) {
+   if ((!fence || pt->ops->timeline_value_str) &&
+   pt->ops->fence_value_str) {
char value[64];
+   bool success;
 
-   parent->ops->pt_value_str(pt, value, sizeof(value));
-   seq_printf(s, ": %s", value);
-   if (fence) {
-   parent->ops->timeline_value_str(parent, value,
-   sizeof(value));
-   seq_printf(s, " / %s", value);
+   pt->ops->fence_value_str(pt, value, sizeof(value));
+   success = strlen(value);
+
+   if (success)
+   seq_printf(s, ": %s", value);
+
+   if (success && fence) {
+   pt->ops->timeline_value_str(pt, value, sizeof(value));
+
+   if (strlen(value))
+   seq_printf(s, " / %s", value);
}
}
 
@@ -138,7 +144,7 @@ static void sync_print_obj(struct seq_file *s, struct 
sync_timeline *obj)
list_for_each(pos, >child_list_head) {
struct sync_pt *pt =
container_of(pos, struct sync_pt, child_list);
-   sync_print_pt(s, pt, false);
+   sync_print_pt(s, >base, false);
}
spin_unlock_irqrestore(>child_list_lock, flags);
 }
@@ -153,11 +159,7 @@ static void sync_print_fence(struct seq_file *s, struct 
sync_fence *fence)
   sync_status_str(atomic_read(>status)));
 
for (i = 0; i < fence->num_fences; ++i) {
-   struct sync_pt *pt =
-   container_of(fence->cbs[i].sync_pt,
-struct sync_pt, base);
-
-   sync_print_pt(s, pt, true);
+   sync_print_pt(s, fence->cbs[i].sync_pt, true);
}
 
spin_lock_irqsave(>wq.lock, flags);
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx