All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Joe Perches <joe@perches.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Sean Paul <seanpaul@chromium.org>,
	David Airlie <airlied@linux.ie>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: Reduce object size of drm_dev_printk/DRM_DEV_<LEVEL> uses
Date: Mon, 25 Sep 2017 15:16:07 +0300	[thread overview]
Message-ID: <87mv5j3reg.fsf@nikula.org> (raw)
In-Reply-To: <693db79230a414560005936cedf4080d20235c03.1506340959.git.joe@perches.com>

On Mon, 25 Sep 2017, Joe Perches <joe@perches.com> wrote:
> Remove unnecessary function_name and prefix arguments.
> Removing these arguments reduces object size.
>
> prefix is used to add an "ERROR" prefix to the format for
> DRM_DEV_ERROR and is an empty string for all other uses.
> This string can be added instead by the DRM_DEV_ERROR macro.
>
> function_name is used to emit the calling function.
> This can be done by using %ps and __builtin_return_address(0).

Did you diff the dmesgs to see how much that gets skewed from __func__
by optimizations?

BR,
Jani.


>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/gpu/drm/drm_drv.c | 12 +++++-------
>  include/drm/drmP.h        | 42 ++++++++++++++++++------------------------
>  include/drm/drm_drv.h     |  5 ++---
>  3 files changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index be38ac7050d4..4d0f2c5197f3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -74,11 +74,8 @@ static bool drm_core_init_complete = false;
>  
>  static struct dentry *drm_debugfs_root;
>  
> -#define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV"
> -
>  void drm_dev_printk(const struct device *dev, const char *level,
> -		    unsigned int category, const char *function_name,
> -		    const char *prefix, const char *format, ...)
> +		    unsigned int category, const char *format, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
> @@ -91,10 +88,11 @@ void drm_dev_printk(const struct device *dev, const char *level,
>  	vaf.va = &args;
>  
>  	if (dev)
> -		dev_printk(level, dev, DRM_PRINTK_FMT, function_name, prefix,
> -			   &vaf);
> +		dev_printk(level, dev, "[" DRM_NAME ":%ps] %pV",
> +			   __builtin_return_address(0), &vaf);
>  	else
> -		printk("%s" DRM_PRINTK_FMT, level, function_name, prefix, &vaf);
> +		printk("%s[" DRM_NAME ":%ps] %pV",
> +		       level, __builtin_return_address(0), &vaf);
>  
>  	va_end(args);
>  }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 7277783a4ff0..16dd63ffa1ff 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -172,8 +172,8 @@ struct pci_controller;
>   * \param arg arguments
>   */
>  #define DRM_DEV_ERROR(dev, fmt, ...)					\
> -	drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
> -		       fmt, ##__VA_ARGS__)
> +	drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE,			\
> +		       "*ERROR* " fmt, ##__VA_ARGS__)
>  #define DRM_ERROR(fmt, ...)						\
>  	drm_printk(KERN_ERR, DRM_UT_NONE, fmt,	##__VA_ARGS__)
>  
> @@ -196,8 +196,8 @@ struct pci_controller;
>  	DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
>  
>  #define DRM_DEV_INFO(dev, fmt, ...)					\
> -	drm_dev_printk(dev, KERN_INFO, DRM_UT_NONE, __func__, "", fmt,	\
> -		       ##__VA_ARGS__)
> +	drm_dev_printk(dev, KERN_INFO, DRM_UT_NONE,			\
> +		       fmt, ##__VA_ARGS__)
>  
>  #define DRM_DEV_INFO_ONCE(dev, fmt, ...)				\
>  ({									\
> @@ -214,50 +214,44 @@ struct pci_controller;
>   * \param fmt printf() like format string.
>   * \param arg arguments
>   */
> -#define DRM_DEV_DEBUG(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt,	\
> -		       ##args)
> +#define DRM_DEV_DEBUG(dev, fmt, ...)					\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG(fmt, ...)						\
>  	drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "",	\
> -		       fmt, ##args)
> +#define DRM_DEV_DEBUG_DRIVER(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG_DRIVER(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_KMS(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt,	\
> -		       ##args)
> +#define DRM_DEV_DEBUG_KMS(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG_KMS(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_PRIME(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "",	\
> -		       fmt, ##args)
> +#define DRM_DEV_DEBUG_PRIME(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, ##__VA_ARGS__)
>  #define DRM_DEBUG_PRIME(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "",	\
> -		       fmt, ##args)
> +#define DRM_DEV_DEBUG_ATOMIC(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG_ATOMIC(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_VBL(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt,	\
> -		       ##args)
> +#define DRM_DEV_DEBUG_VBL(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG_VBL(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
>  
> -#define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)	\
> +#define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, ...)		\
>  ({									\
>  	static DEFINE_RATELIMIT_STATE(_rs,				\
>  				      DEFAULT_RATELIMIT_INTERVAL,	\
>  				      DEFAULT_RATELIMIT_BURST);		\
>  	if (__ratelimit(&_rs))						\
>  		drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ ## level,	\
> -			       __func__, "", fmt, ##args);		\
> +			       fmt, ##__VA_ARGS__);		\
>  })
>  
>  /**
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 71bbaaec836d..f6430f183ca8 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -592,10 +592,9 @@ struct drm_driver {
>  	int dev_priv_size;
>  };
>  
> -__printf(6, 7)
> +__printf(4, 5)
>  void drm_dev_printk(const struct device *dev, const char *level,
> -		    unsigned int category, const char *function_name,
> -		    const char *prefix, const char *format, ...);
> +		    unsigned int category, const char *format, ...);
>  __printf(3, 4)
>  void drm_printk(const char *level, unsigned int category,
>  		const char *format, ...);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Joe Perches <joe@perches.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Sean Paul <seanpaul@chromium.org>,
	David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm: Reduce object size of drm_dev_printk/DRM_DEV_<LEVEL> uses
Date: Mon, 25 Sep 2017 15:16:07 +0300	[thread overview]
Message-ID: <87mv5j3reg.fsf@nikula.org> (raw)
In-Reply-To: <693db79230a414560005936cedf4080d20235c03.1506340959.git.joe@perches.com>

On Mon, 25 Sep 2017, Joe Perches <joe@perches.com> wrote:
> Remove unnecessary function_name and prefix arguments.
> Removing these arguments reduces object size.
>
> prefix is used to add an "ERROR" prefix to the format for
> DRM_DEV_ERROR and is an empty string for all other uses.
> This string can be added instead by the DRM_DEV_ERROR macro.
>
> function_name is used to emit the calling function.
> This can be done by using %ps and __builtin_return_address(0).

Did you diff the dmesgs to see how much that gets skewed from __func__
by optimizations?

BR,
Jani.


>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/gpu/drm/drm_drv.c | 12 +++++-------
>  include/drm/drmP.h        | 42 ++++++++++++++++++------------------------
>  include/drm/drm_drv.h     |  5 ++---
>  3 files changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index be38ac7050d4..4d0f2c5197f3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -74,11 +74,8 @@ static bool drm_core_init_complete = false;
>  
>  static struct dentry *drm_debugfs_root;
>  
> -#define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV"
> -
>  void drm_dev_printk(const struct device *dev, const char *level,
> -		    unsigned int category, const char *function_name,
> -		    const char *prefix, const char *format, ...)
> +		    unsigned int category, const char *format, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
> @@ -91,10 +88,11 @@ void drm_dev_printk(const struct device *dev, const char *level,
>  	vaf.va = &args;
>  
>  	if (dev)
> -		dev_printk(level, dev, DRM_PRINTK_FMT, function_name, prefix,
> -			   &vaf);
> +		dev_printk(level, dev, "[" DRM_NAME ":%ps] %pV",
> +			   __builtin_return_address(0), &vaf);
>  	else
> -		printk("%s" DRM_PRINTK_FMT, level, function_name, prefix, &vaf);
> +		printk("%s[" DRM_NAME ":%ps] %pV",
> +		       level, __builtin_return_address(0), &vaf);
>  
>  	va_end(args);
>  }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 7277783a4ff0..16dd63ffa1ff 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -172,8 +172,8 @@ struct pci_controller;
>   * \param arg arguments
>   */
>  #define DRM_DEV_ERROR(dev, fmt, ...)					\
> -	drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
> -		       fmt, ##__VA_ARGS__)
> +	drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE,			\
> +		       "*ERROR* " fmt, ##__VA_ARGS__)
>  #define DRM_ERROR(fmt, ...)						\
>  	drm_printk(KERN_ERR, DRM_UT_NONE, fmt,	##__VA_ARGS__)
>  
> @@ -196,8 +196,8 @@ struct pci_controller;
>  	DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
>  
>  #define DRM_DEV_INFO(dev, fmt, ...)					\
> -	drm_dev_printk(dev, KERN_INFO, DRM_UT_NONE, __func__, "", fmt,	\
> -		       ##__VA_ARGS__)
> +	drm_dev_printk(dev, KERN_INFO, DRM_UT_NONE,			\
> +		       fmt, ##__VA_ARGS__)
>  
>  #define DRM_DEV_INFO_ONCE(dev, fmt, ...)				\
>  ({									\
> @@ -214,50 +214,44 @@ struct pci_controller;
>   * \param fmt printf() like format string.
>   * \param arg arguments
>   */
> -#define DRM_DEV_DEBUG(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt,	\
> -		       ##args)
> +#define DRM_DEV_DEBUG(dev, fmt, ...)					\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG(fmt, ...)						\
>  	drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "",	\
> -		       fmt, ##args)
> +#define DRM_DEV_DEBUG_DRIVER(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG_DRIVER(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_KMS(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt,	\
> -		       ##args)
> +#define DRM_DEV_DEBUG_KMS(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG_KMS(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_PRIME(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "",	\
> -		       fmt, ##args)
> +#define DRM_DEV_DEBUG_PRIME(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, ##__VA_ARGS__)
>  #define DRM_DEBUG_PRIME(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "",	\
> -		       fmt, ##args)
> +#define DRM_DEV_DEBUG_ATOMIC(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG_ATOMIC(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>  
> -#define DRM_DEV_DEBUG_VBL(dev, fmt, args...)				\
> -	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt,	\
> -		       ##args)
> +#define DRM_DEV_DEBUG_VBL(dev, fmt, ...)				\
> +	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
>  #define DRM_DEBUG_VBL(fmt, ...)					\
>  	drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
>  
> -#define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)	\
> +#define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, ...)		\
>  ({									\
>  	static DEFINE_RATELIMIT_STATE(_rs,				\
>  				      DEFAULT_RATELIMIT_INTERVAL,	\
>  				      DEFAULT_RATELIMIT_BURST);		\
>  	if (__ratelimit(&_rs))						\
>  		drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ ## level,	\
> -			       __func__, "", fmt, ##args);		\
> +			       fmt, ##__VA_ARGS__);		\
>  })
>  
>  /**
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 71bbaaec836d..f6430f183ca8 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -592,10 +592,9 @@ struct drm_driver {
>  	int dev_priv_size;
>  };
>  
> -__printf(6, 7)
> +__printf(4, 5)
>  void drm_dev_printk(const struct device *dev, const char *level,
> -		    unsigned int category, const char *function_name,
> -		    const char *prefix, const char *format, ...);
> +		    unsigned int category, const char *format, ...);
>  __printf(3, 4)
>  void drm_printk(const char *level, unsigned int category,
>  		const char *format, ...);

-- 
Jani Nikula, Intel Open Source Technology Center

  reply	other threads:[~2017-09-25 12:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-25 12:03 [PATCH] drm: Reduce object size of drm_dev_printk/DRM_DEV_<LEVEL> uses Joe Perches
2017-09-25 12:16 ` Jani Nikula [this message]
2017-09-25 12:16   ` Jani Nikula
2017-09-25 12:28   ` 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=87mv5j3reg.fsf@nikula.org \
    --to=jani.nikula@linux.intel.com \
    --cc=airlied@linux.ie \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seanpaul@chromium.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.