linux-kernel - Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180315150524.GS5453@intel.com>
Date:   Thu, 15 Mar 2018 17:05:24 +0200
From:   Ville Syrjälä <ville.syrjala@...ux.intel.com>
To:     Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
Cc:     Joe Perches <joe@...ches.com>,
        Gustavo Padovan <gustavo@...ovan.org>,
        Sean Paul <seanpaul@...omium.org>,
        David Airlie <airlied@...ux.ie>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses

On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> >> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> >> arguments that can be removed by creating separate functins.
> >>
> >> Create specific functions for these calls to reduce x86/64 defconfig
> >> size by ~20k.
> >>
> >> Modify the existing macros to use the specific calls.
> >>
> >> new:
> >> $ size -t drivers/gpu/drm/built-in.a | tail -1
> >> 1876562	  44542	    995	1922099	 1d5433	(TOTALS)
> >>
> >> old:
> >> $ size -t drivers/gpu/drm/built-in.a | tail -1
> >> 1897565	  44542	    995	1943102	 1da63e	(TOTALS)
> >>
> >> Miscellanea:
> >>
> >> o intel_display requires a change to use the specific calls.
> > How much would we lose if we move the (drm_debug&FOO) outside the
> > functions again? I'm somewhat concerned about all the function call
> > overhead when debugs aren't even enabled.
> 
> Upstream:
>    text    data     bss     dec     hex filename
>  377143    5689    4352  387184   5e870 drivers/gpu/drm/drm.ko
> 
> With this patch:
>  373831    5689    4352  383872   5db80 drivers/gpu/drm/drm.ko
> 
> Moving the if outside (below):
>  377629    5689    4352  387670   5ea56 drivers/gpu/drm/drm.ko
> 
> Bye savings..
> 
> I don't think there are any places in which the debug output is performance sensitive,
> so I'm ok with not inlining.

Not performance sensitive as such perhaps. But pointlessly wasting cpu
cycles for nop function calls isn't particularly great. Would be nice
to actually measure how much overhead there is on some weaker systems.
IIRC older Atoms were particularly bad at this stuff.

> ---
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 79abf6d5b4db..928822403a59 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,14 +89,11 @@ void drm_dev_printk(const struct device *dev, const char *level,
>  }
>  EXPORT_SYMBOL(drm_dev_printk);
>  
> -void drm_dbg(unsigned int category, const char *format, ...)
> +void __drm_dbg(const char *format, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
>  
> -	if (!(drm_debug & category))
> -		return;
> -
>  	va_start(args, format);
>  	vaf.fmt = format;
>  	vaf.va = &args;
> @@ -106,7 +103,7 @@ void drm_dbg(unsigned int category, const char *format, ...)
>  
>  	va_end(args);
>  }
> -EXPORT_SYMBOL(drm_dbg);
> +EXPORT_SYMBOL(__drm_dbg);
>  
>  void drm_err(const char *format, ...)
>  {
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 3a40c5a3a5fa..2a145b97bdfc 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -200,8 +200,17 @@ __printf(6, 7)
>  void drm_dev_printk(const struct device *dev, const char *level,
>  		    unsigned int category, const char *function_name,
>  		    const char *prefix, const char *format, ...);
> -__printf(2, 3)
> -void drm_dbg(unsigned int category, const char *format, ...);
> +
> +__printf(1, 2)
> +void __drm_dbg(const char *format, ...);
> +
> +
> +#define drm_dbg(category, format, ...) \
> +	do {	\
> +		if (drm_debug & category)	\
> +			__drm_dbg(format, ## __VA_ARGS__);	\
> +	} while (0)
> +
>  __printf(1, 2)
>  void drm_err(const char *format, ...);
>  

-- 
Ville Syrjälä
Intel OTC