On 12/11/2015 05:11 AM, John.C.Harrison@xxxxxxxxx wrote:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> The sync code has a facility for dumping current state information via
> debugfs. It also has a way to re-use the same code for dumping to the
> kernel log on an internal error. However, the redirection was rather
> clunky and split the output across multiple prints at arbitrary
> boundaries. This made it difficult to read and could result in output
> from different sources being randomly interspersed.
>
> This patch improves the redirection code to split the output on line
> feed boundaries instead. It also adds support for highlighting the
> offending fence object that caused the state dump in the first place.
>
> v4: New patch in series.
>
> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
> ---
> drivers/android/sync.c | 9 ++++++--
> drivers/android/sync.h | 5 +++--
> drivers/android/sync_debug.c | 50 ++++++++++++++++++++++++++++++++------------
> 3 files changed, 47 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/android/sync.c b/drivers/android/sync.c
> index 7f0e919..db4a54b 100644
> --- a/drivers/android/sync.c
> +++ b/drivers/android/sync.c
> @@ -86,6 +86,11 @@ static void sync_timeline_put(struct sync_timeline *obj)
>
> void sync_timeline_destroy(struct sync_timeline *obj)
> {
> + if (!list_empty(&obj->active_list_head)) {
> + pr_info("destroying timeline with outstanding fences!\n");
> + sync_dump_timeline(obj);
> + }
> +
> obj->destroyed = true;
> /*
> * Ensure timeline is marked as destroyed before
> @@ -397,7 +402,7 @@ int sync_fence_wait(struct sync_fence *fence, long timeout)
> if (timeout) {
> pr_info("fence timeout on [%p] after %dms\n", fence,
> jiffies_to_msecs(timeout));
> - sync_dump();
> + sync_dump(fence);
> }
> return -ETIME;
> }
> @@ -405,7 +410,7 @@ int sync_fence_wait(struct sync_fence *fence, long timeout)
> ret = atomic_read(&fence->status);
> if (ret) {
> pr_info("fence error %ld on [%p]\n", ret, fence);
> - sync_dump();
> + sync_dump(fence);
> }
> return ret;
> }
> diff --git a/drivers/android/sync.h b/drivers/android/sync.h
> index 4ccff01..d57fa0a 100644
> --- a/drivers/android/sync.h
> +++ b/drivers/android/sync.h
> @@ -351,14 +351,15 @@ void sync_timeline_debug_add(struct sync_timeline *obj);
> void sync_timeline_debug_remove(struct sync_timeline *obj);
> void sync_fence_debug_add(struct sync_fence *fence);
> void sync_fence_debug_remove(struct sync_fence *fence);
> -void sync_dump(void);
> +void sync_dump(struct sync_fence *fence);
> +void sync_dump_timeline(struct sync_timeline *timeline);
>
> #else
> # define sync_timeline_debug_add(obj)
> # define sync_timeline_debug_remove(obj)
> # define sync_fence_debug_add(fence)
> # define sync_fence_debug_remove(fence)
> -# define sync_dump()
> +# define sync_dump(fence)
> #endif
> int sync_fence_wake_up_wq(wait_queue_t *curr, unsigned mode,
> int wake_flags, void *key);
> diff --git a/drivers/android/sync_debug.c b/drivers/android/sync_debug.c
> index f45d13c..9b87e0a 100644
> --- a/drivers/android/sync_debug.c
> +++ b/drivers/android/sync_debug.c
> @@ -229,28 +229,52 @@ late_initcall(sync_debugfs_init);
>
> #define DUMP_CHUNK 256
> static char sync_dump_buf[64 * 1024];
> -void sync_dump(void)
> +
> +static void sync_dump_dfs(struct seq_file *s, void *targetPtr)
> +{
> + char *start, *end;
> + char targetStr[100];
> +
> + if (targetPtr)
> + snprintf(targetStr, sizeof(targetStr) - 1, "%p", targetPtr);
> +
> + start = end = s->buf;
> + while( (end = strchr(end, '\n'))) {
> + *end = 0;
> + if (targetPtr && strstr(start, targetStr))
> + pr_info("*** %s ***\n", start);
> + else
> + pr_info("%s\n", start);
> + start = ++end;
> + }
> +
> + if ((start - s->buf) < s->count)
> + pr_info("%d vs %d: >?>%s<?<\n", (uint32_t) (start - s->buf), (uint32_t) s->count, start);
> +}
> +
> +void sync_dump(struct sync_fence *targetPtr)
> {
> struct seq_file s = {
> .buf = sync_dump_buf,
> .size = sizeof(sync_dump_buf) - 1,
> };
> - int i;
>
> sync_debugfs_show(&s, NULL);
>
> - for (i = 0; i < s.count; i += DUMP_CHUNK) {
> - if ((s.count - i) > DUMP_CHUNK) {
> - char c = s.buf[i + DUMP_CHUNK];
> + sync_dump_dfs(&s, targetPtr);
> +}
>
> - s.buf[i + DUMP_CHUNK] = 0;
> - pr_cont("%s", s.buf + i);
> - s.buf[i + DUMP_CHUNK] = c;
> - } else {
> - s.buf[s.count] = 0;
> - pr_cont("%s", s.buf + i);
> - }
> - }
> +void sync_dump_timeline(struct sync_timeline *timeline)
> +{
> + struct seq_file s = {
> + .buf = sync_dump_buf,
> + .size = sizeof(sync_dump_buf) - 1,
> + };
> +
> + pr_info("timeline: %p\n", timeline);
> + sync_print_obj(&s, timeline);
> +
> + sync_dump_dfs(&s, NULL);
> }
>
> #endif
>
I guess the Android guys might have feedback here, but it seems fine to me.
Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
