On 12/11/2015 05:11 AM, John.C.Harrison@xxxxxxxxx wrote:
> From: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Riley Andrews <riandrews@xxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Arve Hjønnevåg <arve@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxx>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
