All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ian Romanick <idr@freedesktop.org>
To: Joe Perches <joe@perches.com>
Cc: David Airlie <airlied@linux.ie>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm: Create and use drm_err
Date: Mon, 18 Apr 2011 15:56:49 -0700	[thread overview]
Message-ID: <4DACC1B1.8090808@freedesktop.org> (raw)
In-Reply-To: <d70d2c8ad548f99e8d028a2fbd4c5b35609f8917.1303097621.git.joe@perches.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/17/2011 08:35 PM, Joe Perches wrote:
> Reduce drm text size ~1% by using drm_err and
> printf extension %pV to emit error messages.
> 
> Remove unused macro DRM_MEM_ERROR.
> 
> $ size drivers/gpu/drm/built-in.o*
>    text	   data	    bss	    dec	    hex	filename
>  361159	   9663	    256	 371078	  5a986	drivers/gpu/drm/built-in.o.new
>  365416	   9663	    256	 375335	  5ba27	drivers/gpu/drm/built-in.o.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/gpu/drm/drm_stub.c |   21 +++++++++++++++++++++
>  include/drm/drmP.h         |   21 +++++++--------------
>  2 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 001273d..6d7b083 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -62,6 +62,26 @@ struct idr drm_minors_idr;
>  struct class *drm_class;
>  struct proc_dir_entry *drm_proc_root;
>  struct dentry *drm_debugfs_root;
> +
> +int drm_err(const char *func, const char *format, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +	int r;
> +
> +	va_start(args, format);
> +
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +
> +	r = printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* %pV", func, &vaf);

This code has ben reorganized a lot over the years, so this comment may
be bogus.  However...

I believe that DRM_NAME is a define, and drm_stub.c is in common code.
As a result, won't this change cause something different to get logged?

> +
> +	va_end(args);
> +
> +	return r;
> +}
> +EXPORT_SYMBOL(drm_err);
> +
>  void drm_ut_debug_printk(unsigned int request_level,
>  			 const char *prefix,
>  			 const char *function_name,
> @@ -78,6 +98,7 @@ void drm_ut_debug_printk(unsigned int request_level,
>  	}
>  }
>  EXPORT_SYMBOL(drm_ut_debug_printk);
> +
>  static int drm_minor_get_id(struct drm_device *dev, int type)
>  {
>  	int new_id;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 202424d..22db51d 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -126,6 +126,9 @@ extern void drm_ut_debug_printk(unsigned int request_level,
>  				const char *prefix,
>  				const char *function_name,
>  				const char *format, ...);
> +extern __attribute__((format (printf, 2, 3)))
> +int drm_err(const char *func, const char *format, ...);
> +
>  /***********************************************************************/
>  /** \name DRM template customization defaults */
>  /*@{*/
> @@ -181,21 +184,11 @@ extern void drm_ut_debug_printk(unsigned int request_level,
>   * \param fmt printf() like format string.
>   * \param arg arguments
>   */
> -#define DRM_ERROR(fmt, arg...) \
> -	printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* " fmt , __func__ , ##arg)
> -
> -/**
> - * Memory error output.
> - *
> - * \param area memory area where the error occurred.
> - * \param fmt printf() like format string.
> - * \param arg arguments
> - */
> -#define DRM_MEM_ERROR(area, fmt, arg...) \
> -	printk(KERN_ERR "[" DRM_NAME ":%s:%s] *ERROR* " fmt , __func__, \
> -	       drm_mem_stats[area].name , ##arg)
> +#define DRM_ERROR(fmt, ...)				\
> +	drm_err(__func__, fmt, ##__VA_ARGS__)
>  
> -#define DRM_INFO(fmt, arg...)  printk(KERN_INFO "[" DRM_NAME "] " fmt , ##arg)
> +#define DRM_INFO(fmt, ...)				\
> +	printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
>  
>  /**
>   * Debug output.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk2swbEACgkQX1gOwKyEAw+w/ACfd2QMJWOQU5f9sgavPXkfPPfW
GygAn2D8bjKkIV8wOXYC1fOI9w4DPWHj
=q3FM
-----END PGP SIGNATURE-----

WARNING: multiple messages have this Message-ID (diff)
From: Ian Romanick <idr@freedesktop.org>
To: Joe Perches <joe@perches.com>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm: Create and use drm_err
Date: Mon, 18 Apr 2011 15:56:49 -0700	[thread overview]
Message-ID: <4DACC1B1.8090808@freedesktop.org> (raw)
In-Reply-To: <d70d2c8ad548f99e8d028a2fbd4c5b35609f8917.1303097621.git.joe@perches.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/17/2011 08:35 PM, Joe Perches wrote:
> Reduce drm text size ~1% by using drm_err and
> printf extension %pV to emit error messages.
> 
> Remove unused macro DRM_MEM_ERROR.
> 
> $ size drivers/gpu/drm/built-in.o*
>    text	   data	    bss	    dec	    hex	filename
>  361159	   9663	    256	 371078	  5a986	drivers/gpu/drm/built-in.o.new
>  365416	   9663	    256	 375335	  5ba27	drivers/gpu/drm/built-in.o.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/gpu/drm/drm_stub.c |   21 +++++++++++++++++++++
>  include/drm/drmP.h         |   21 +++++++--------------
>  2 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 001273d..6d7b083 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -62,6 +62,26 @@ struct idr drm_minors_idr;
>  struct class *drm_class;
>  struct proc_dir_entry *drm_proc_root;
>  struct dentry *drm_debugfs_root;
> +
> +int drm_err(const char *func, const char *format, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +	int r;
> +
> +	va_start(args, format);
> +
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +
> +	r = printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* %pV", func, &vaf);

This code has ben reorganized a lot over the years, so this comment may
be bogus.  However...

I believe that DRM_NAME is a define, and drm_stub.c is in common code.
As a result, won't this change cause something different to get logged?

> +
> +	va_end(args);
> +
> +	return r;
> +}
> +EXPORT_SYMBOL(drm_err);
> +
>  void drm_ut_debug_printk(unsigned int request_level,
>  			 const char *prefix,
>  			 const char *function_name,
> @@ -78,6 +98,7 @@ void drm_ut_debug_printk(unsigned int request_level,
>  	}
>  }
>  EXPORT_SYMBOL(drm_ut_debug_printk);
> +
>  static int drm_minor_get_id(struct drm_device *dev, int type)
>  {
>  	int new_id;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 202424d..22db51d 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -126,6 +126,9 @@ extern void drm_ut_debug_printk(unsigned int request_level,
>  				const char *prefix,
>  				const char *function_name,
>  				const char *format, ...);
> +extern __attribute__((format (printf, 2, 3)))
> +int drm_err(const char *func, const char *format, ...);
> +
>  /***********************************************************************/
>  /** \name DRM template customization defaults */
>  /*@{*/
> @@ -181,21 +184,11 @@ extern void drm_ut_debug_printk(unsigned int request_level,
>   * \param fmt printf() like format string.
>   * \param arg arguments
>   */
> -#define DRM_ERROR(fmt, arg...) \
> -	printk(KERN_ERR "[" DRM_NAME ":%s] *ERROR* " fmt , __func__ , ##arg)
> -
> -/**
> - * Memory error output.
> - *
> - * \param area memory area where the error occurred.
> - * \param fmt printf() like format string.
> - * \param arg arguments
> - */
> -#define DRM_MEM_ERROR(area, fmt, arg...) \
> -	printk(KERN_ERR "[" DRM_NAME ":%s:%s] *ERROR* " fmt , __func__, \
> -	       drm_mem_stats[area].name , ##arg)
> +#define DRM_ERROR(fmt, ...)				\
> +	drm_err(__func__, fmt, ##__VA_ARGS__)
>  
> -#define DRM_INFO(fmt, arg...)  printk(KERN_INFO "[" DRM_NAME "] " fmt , ##arg)
> +#define DRM_INFO(fmt, ...)				\
> +	printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
>  
>  /**
>   * Debug output.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk2swbEACgkQX1gOwKyEAw+w/ACfd2QMJWOQU5f9sgavPXkfPPfW
GygAn2D8bjKkIV8wOXYC1fOI9w4DPWHj
=q3FM
-----END PGP SIGNATURE-----

  reply	other threads:[~2011-04-18 23:02 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-18  3:35 [PATCH 0/2] drm: Message logging cleanups Joe Perches
2011-04-18  3:35 ` [PATCH 1/2] drm: Create and use drm_err Joe Perches
2011-04-18 22:56   ` Ian Romanick [this message]
2011-04-18 22:56     ` Ian Romanick
2011-04-18 23:00     ` Dave Airlie
2011-04-18 23:05     ` Joe Perches
2011-04-18  3:35 ` [PATCH 2/2] drm: Verify debug message arguments Joe Perches
2011-04-18 23:01   ` Ian Romanick
2011-04-18 23:01     ` Ian Romanick
2011-04-18 23:09     ` Joe Perches
2011-04-19 16:26       ` Marcin Slusarz
2011-04-19 16:31         ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DACC1B1.8090808@freedesktop.org \
    --to=idr@freedesktop.org \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.