Re: [PATCH 02/13] drm: drm_printer: Add printer for devcoredump

From: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
To: Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 02/13] drm: drm_printer: Add printer for devcoredump
Date: Thu, 12 Jul 2018 20:40:55 +0100	[thread overview]
Message-ID: <153142445520.24385.8265178741409388476@skylake-alporthouse-com> (raw)
In-Reply-To: <20180712185930.2492-3-jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Quoting Jordan Crouse (2018-07-12 19:59:19)
> Add a drm printer suitable for use with the read callback for
> devcoredump or other suitable buffer based output format that
> isn't otherwise covered by seq_file.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  drivers/gpu/drm/drm_print.c | 74 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_print.h     | 27 ++++++++++++++
>  2 files changed, 101 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index b25f98f33f6c..03d1f98e5ac7 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -30,6 +30,80 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_print.h>
>  
> +void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
> +{
> +       struct drm_print_iterator *iterator = p->arg;
> +       ssize_t len;
> +
> +       if (!iterator->remain)
> +               return;
> +
> +       /* Figure out how big the string will be */
> +       len = snprintf(NULL, 0, "%pV", vaf);

I was thinking there's some duplication here (kmalloc + snprintf) that
could be reduced to kasprintf here. Is avoiding that allocation
important or frequent enough to merit open coding?

It's pity the kernel's printk doesn't support %n, so that leaves with

buf = kasprintf(GFP_... , "%pV", vaf);
if (!buf)
	return;

len = strlen(buf);

and even the copy + increment looks like it can then be factored to share
more code.
-Chris
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno