Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 15 Mar 2018 18:14:34 +0200
From:   Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
To:     Joe Perches <joe@perches.com>
Cc:     Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Gustavo Padovan <gustavo@padovan.org>,
        Sean Paul <seanpaul@chromium.org>,
        David Airlie <airlied@linux.ie>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
Message-ID: <20180315161434.GV5453@intel.com>
References: <016b5cb84cede20fd0f91ed6965421d99fd5f2ce.1520978414.git.joe@perches.com>
 <20180315133026.GR5453@intel.com>
 <1b50f5d8-97a6-2442-34bb-2782c35505fd@linux.intel.com>
 <20180315150524.GS5453@intel.com>
 <1521127073.22221.13.camel@perches.com>
 <20180315153734.GT5453@intel.com>
 <1521128645.22221.18.camel@perches.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1521128645.22221.18.camel@perches.com>
User-Agent: Mutt/1.7.2 (2016-11-26)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 15, 2018 at 08:44:05AM -0700, Joe Perches wrote:
> On Thu, 2018-03-15 at 17:37 +0200, Ville Syrj?l? wrote:
> > On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote:
> > > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrj?l? wrote:
> > > > 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?
> > > 
> > > again?
> > 
> > We used to do that. Someone changed it a while back, unintentially
> > I believe.
> > 
> > > 
> > > > > >  I'm somewhat concerned about all the function call
> > > > > > overhead when debugs aren't even enabled.
> > > 
> > > Perhaps better to have compilation elimination
> > > of the entire debug output instead.
> > 
> > That would require every bug reporter to recompile the kernel first.
> > So this is not a solution we would ever seriously consider.
> > 
> > Not sure if it would be possible to use the alternatives thing to
> > eliminate the function calls unless the user boots wih drm.debug!=0?
> > 
> > > 
> > > I think you are discussing a different issue and
> > > this discussion should not block this patch as
> > > this patch has no impact other than code size
> > > reduction.
> > 
> > But what is the goal of the code size reduction?
> 
> Smaller code.
> 
> > I assume the main
> > goal is to make better use of the instruction cache to make the
> > code faster. If there's a tradeoff between smaller and slightly
> > faster vs. larger and a singificantly faster I tend to think we
> > should go for the latter option.
> 
> There's no trade-off in this patch for faster/larger.
> This patch is simply smaller.  Smaller is better.

This feels a bit like saying pink is better than red because it's
more pink.

That said, I'm not arguing against this patch as such. Making things
smaller "just because" usually doesn't cause problems. But I was
hoping that we might be after some more tangible gains here, and
thus pointed out that there may be a better way to achieve even
bigger gains.

-- 
Ville Syrj?l?
Intel OTC