From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 3/3] xen/init: Annotate all command line parameter infrastructure as const Date: Tue, 9 Feb 2016 14:40:01 +0000 Message-ID: <56B9FA41.5020705@citrix.com> References: <1454951267-30034-1-git-send-email-andrew.cooper3@citrix.com> <1454951267-30034-3-git-send-email-andrew.cooper3@citrix.com> <56B9ECE502000078000D00CC@prv-mh.provo.novell.com> <56B9EF1D.9010604@citrix.com> <56BA068202000078000D018B@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56BA068202000078000D018B@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 09/02/16 14:32, Jan Beulich wrote: >>>> On 09.02.16 at 14:52, wrote: >> On 09/02/16 12:43, Jan Beulich wrote: >>>>>> On 08.02.16 at 18:07, wrote: >>>> --- a/xen/arch/x86/xen.lds.S >>>> +++ b/xen/arch/x86/xen.lds.S >>>> @@ -120,6 +120,12 @@ SECTIONS >>>> .init.data : { >>>> *(.init.rodata) >>>> *(.init.rodata.str*) >>>> + >>>> + . = ALIGN(32); >>> Why 32? >>> >>>> + __setup_start = .; >>>> + *(.init.setup) >>>> + __setup_end = .; >>>> + >>>> *(.init.data) >>>> *(.init.data.rel) >>>> *(.init.data.rel.*) >>>> @@ -146,11 +152,6 @@ SECTIONS >>>> __ctors_end = .; >>>> } :text >>>> . = ALIGN(32); >>>> - .init.setup : { >>>> - __setup_start = .; >>>> - *(.init.setup) >>>> - __setup_end = .; >>>> - } :text >>> If just because it was 32 here, I don't think that's a compelling >>> reason. With it (above, not necessarily here) reduced to 8 (which >>> of course could also be done while committing, if you agree and >>> there are no deeper reasons), >>> Reviewed-by: Jan Beulich >> It was just because of the code here. I still can't think of any >> specific reason why 32 is needed, so the ALIGN() can just be dropped. > No, dropping ALIGN() altogether would make the placement of > __setup_start dependent upon the alignment of the previous > section (and since it's a strings section which precedes it, > problems would be quite likely). (This is, btw., why it would be > better if we used __startof__ and __alignof__ instead of linker > script generated symbols. I may give that a try ...) Actually, thinking about it, andrewcoop@andrewcoop:/local/xen.git/xen$ pahole xen-syms -C kernel_param struct kernel_param { const char * name; /* 0 8 */ enum { OPT_STR = 0, OPT_UINT = 1, OPT_BOOL = 2, OPT_SIZE = 3, OPT_CUSTOM = 4, } type; /* 8 4 */ /* XXX 4 bytes hole, try to pack */ void * var; /* 16 8 */ unsigned int len; /* 24 4 */ /* size: 32, cachelines: 1, members: 4 */ /* sum members: 24, holes: 1, sum holes: 4 */ /* padding: 4 */ /* last cacheline: 32 bytes */ }; This will be where the 32 byte alignment comes from, although dropping the structure size to 24 bytes would be trivial. ~Andrew