Re: [Xen-devel] [PATCH 3/3] xen/init: Annotate all command line parameter infrastructure as const

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.02.16 at 15:40, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 09/02/16 14:32, Jan Beulich wrote:
>>>>> On 09.02.16 at 14:52, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 09/02/16 12:43, Jan Beulich wrote:
>>>>>>> On 08.02.16 at 18:07, <andrew.cooper3@xxxxxxxxxx> 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 <jbeulich@xxxxxxxx>
>>> 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,

But that's the structure _size_, unrelated to any needed alignment.

> although dropping
> the structure size to 24 bytes would be trivial.

Patch already done (to be re-based on top of yours).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel