drm: Don't split up debug output
diff mbox

Message ID 1384723502-13398-1-git-send-email-daniel.vetter@ffwll.ch
State Accepted
Headers show

Commit Message

Daniel Vetter Nov. 17, 2013, 9:25 p.m. UTC
Otherwise we risk that the 2nd part of the line ends up on a line of
it's own, which means a kernel dmesg line without a log level. This
then upsets the dmesg checker in piglit.

Only really happens in some of the truly nasty igt testcases which
race cache dropping (through debugfs) with other gem operations.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_stub.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Thierry Reding Nov. 18, 2013, 10:55 a.m. UTC | #1
On Sun, Nov 17, 2013 at 10:25:02PM +0100, Daniel Vetter wrote:
> Otherwise we risk that the 2nd part of the line ends up on a line of
> it's own, which means a kernel dmesg line without a log level. This
> then upsets the dmesg checker in piglit.
> 
> Only really happens in some of the truly nasty igt testcases which
> race cache dropping (through debugfs) with other gem operations.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_stub.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index f53d5246979c..74e0357c1c38 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -99,13 +99,19 @@ void drm_ut_debug_printk(unsigned int request_level,
>  			 const char *function_name,
>  			 const char *format, ...)
>  {
> +	struct va_format vaf;
>  	va_list args;
>  
>  	if (drm_debug & request_level) {
> -		if (function_name)
> -			printk(KERN_DEBUG "[%s:%s], ", prefix, function_name);
>  		va_start(args, format);
> -		vprintk(format, args);
> +		vaf.fmt = format;
> +		vaf.va = &args;
> +
> +		if (function_name)
> +			printk(KERN_DEBUG "[%s:%s], %pV", prefix,
> +			       function_name, &vaf);
> +		else
> +			printk(KERN_DEBUG "%pV", &vaf);
>  		va_end(args);
>  	}
>  }

According to Documentation/printk-formats.txt, usage of %pV is not
recommended unless the format string and va_list can be properly
verified for correctness.

I guess we leave it up to the compiler to do that verification using the
__printf() annotation, so I think it should be fine to use it here:

Reviewed-by: Thierry Reding <treding@nvidia.com>

Patch
diff mbox

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index f53d5246979c..74e0357c1c38 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -99,13 +99,19 @@  void drm_ut_debug_printk(unsigned int request_level,
 			 const char *function_name,
 			 const char *format, ...)
 {
+	struct va_format vaf;
 	va_list args;
 
 	if (drm_debug & request_level) {
-		if (function_name)
-			printk(KERN_DEBUG "[%s:%s], ", prefix, function_name);
 		va_start(args, format);
-		vprintk(format, args);
+		vaf.fmt = format;
+		vaf.va = &args;
+
+		if (function_name)
+			printk(KERN_DEBUG "[%s:%s], %pV", prefix,
+			       function_name, &vaf);
+		else
+			printk(KERN_DEBUG "%pV", &vaf);
 		va_end(args);
 	}
 }