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

[Andrew Cooper <andrew.cooper3@citrix.com>]

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