Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932108AbdHYSNx (ORCPT ); Fri, 25 Aug 2017 14:13:53 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:41089 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755518AbdHYSNw (ORCPT ); Fri, 25 Aug 2017 14:13:52 -0400 Subject: Re: [PATCH] x86/platform/intel-mid: make several arrays static, makes code smaller To: Andy Shevchenko , Lukas Wunner References: <20170825163206.23250-1-colin.king@canonical.com> <20170825175104.GA19150@wunner.de> <1503683627.25945.114.camel@linux.intel.com> Cc: Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , x86@kernel.org, Bjorn Helgaas , kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org From: Colin Ian King Message-ID: Date: Fri, 25 Aug 2017 19:13:49 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1503683627.25945.114.camel@linux.intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1830 Lines: 45 On 25/08/17 18:53, Andy Shevchenko wrote: > On Fri, 2017-08-25 at 19:51 +0200, Lukas Wunner wrote: >> On Fri, Aug 25, 2017 at 05:32:06PM +0100, Colin King wrote: >>> --- a/arch/x86/platform/intel-mid/pwr.c >>> +++ b/arch/x86/platform/intel-mid/pwr.c >>> @@ -444,7 +444,7 @@ static int mid_set_initial_state(struct mid_pwr >>> *pwr, const u32 *states) >>> static int pnw_set_initial_state(struct mid_pwr *pwr) >>> { >>> /* On Penwell SRAM must stay powered on */ >>> - const u32 states[] = { >>> + static const u32 states[] = { >>> 0xf00fffff, /* PM_SSC(0) */ >>> 0xffffffff, /* PM_SSC(1) */ >>> 0xffffffff, /* PM_SSC(2) */ >> >> That's a known gcc bug: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725 >> >> There are hundreds of constant compound literals that are generated >> on the stack rather than stored in rodata, do you intend to file >> patches for all of them? Adding static everywhere is just a >> workaround that bloats the code. Fixing the root cause in gcc would >> make more sense. Hrm. constifying will specify it won't be changed; section A8.2 of "The C programming language" 2nd edition states that this specifier "is to announce that objects may be placed in read-only memory, and perhaps increase opportunities for optimization". Emphasis on "may", it can do, but it does not necessarily have to. The static storage class specifier will ensure the object is not on the stack for sure. So I think from my understanding of the semantics described in K&R 2nd edition: const - data is read only, and can be put into read-only location if compiler can (or wants) to do so static - data will be not stashed on the stack. So my change is to be totally clear on the intent - make it read only and ensure it is not populated at run time on the stack. Colin > > That is a good point. >