diff mbox

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

Message ID 1449839521-21958-2-git-send-email-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

John Harrison Dec. 11, 2015, 1:11 p.m. UTC
From: Maarten Lankhorst <maarten.lankhorst@canonical.com>

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 <maarten.lankhorst@canonical.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: devel@driverdev.osuosl.org
Cc: Riley Andrews <riandrews@android.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Arve Hjønnevåg <arve@android.com>
---
 drivers/staging/android/sync_debug.c | 42 +++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 20 deletions(-)

Comments

Jesse Barnes Dec. 17, 2015, 5:32 p.m. UTC | #1
On 12/11/2015 05:11 AM, John.C.Harrison@Intel.com wrote:
> From: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> 
> 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 <maarten.lankhorst@canonical.com>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: devel@driverdev.osuosl.org
> Cc: Riley Andrews <riandrews@android.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Arve Hjønnevåg <arve@android.com>
> ---
>  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(&pt->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, &obj->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, &pt->base, false);
>  	}
>  	spin_unlock_irqrestore(&obj->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(&fence->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(&fence->wq.lock, flags);
> 

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
diff mbox

Patch

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(&pt->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, &obj->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, &pt->base, false);
 	}
 	spin_unlock_irqrestore(&obj->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(&fence->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(&fence->wq.lock, flags);