Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580635 > unrolled thread

Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Borislav Petkov <bp@alien8.de> - 2017-02-14 17:20 +0100
    Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Mathias Krause <minipli@googlemail.com> - 2017-02-14 17:50 +0100
    Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Geert Uytterhoeven <geert@linux-m68k.org> - 2017-02-14 19:00 +0100
      Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 Borislav Petkov <bp@alien8.de> - 2017-02-14 19:30 +0100
      Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86 "H. Peter Anvin" <hpa@zytor.com> - 2017-02-14 19:50 +0100

#1580635 — Re: [PATCH 1/6] x86: drop unneded members of struct cpuinfo_x86

On Sun, Feb 12, 2017 at 10:12:07PM +0100, Mathias Krause wrote:
> Those member serve no purpose -- not even fill padding for alignment or
> such. So just get rid of them.

Well, almost. You need the wp_works_ok removal patch too, otherwise you
have the 3 bytes hole below.

But the wp_works_ok goes away too so I guess that's fine.

$ pahole -C cpuinfo_x86 vmlinux
struct cpuinfo_x86 {
        __u8                       x86;                  /*     0     1 */
        __u8                       x86_vendor;           /*     1     1 */
        __u8                       x86_model;            /*     2     1 */
        __u8                       x86_mask;             /*     3     1 */
        char                       wp_works_ok;          /*     4     1 */
        __u8                       x86_virt_bits;        /*     5     1 */
        __u8                       x86_phys_bits;        /*     6     1 */
        __u8                       x86_coreid_bits;      /*     7     1 */
        __u8                       cu_id;                /*     8     1 */

        /* XXX 3 bytes hole, try to pack */

        __u32                      extended_cpuid_level; /*    12     4 */
        int                        cpuid_level;          /*    16     4 */
        __u32                      x86_capability[19];   /*    20    76 */
        /* --- cacheline 1 boundary (64 bytes) was 32 bytes ago --- */
        char                       x86_vendor_id[16];    /*    96    16 */
        char                       x86_model_id[64];     /*   112    64 */
        /* --- cacheline 2 boundary (128 bytes) was 48 bytes ago --- */
        int                        x86_cache_size;       /*   176     4 */
        int                        x86_cache_alignment;  /*   180     4 */
        int                        x86_cache_max_rmid;   /*   184     4 */
        int                        x86_cache_occ_scale;  /*   188     4 */
        /* --- cacheline 3 boundary (192 bytes) --- */
        int                        x86_power;            /*   192     4 */
        long unsigned int          loops_per_jiffy;      /*   196     4 */
        u16                        x86_max_cores;        /*   200     2 */
        u16                        apicid;               /*   202     2 */
        u16                        initial_apicid;       /*   204     2 */
        u16                        x86_clflush_size;     /*   206     2 */
        u16                        booted_cores;         /*   208     2 */
        u16                        phys_proc_id;         /*   210     2 */
        u16                        logical_proc_id;      /*   212     2 */
        u16                        cpu_core_id;          /*   214     2 */
        u16                        cpu_index;            /*   216     2 */

        /* XXX 2 bytes hole, try to pack */

        u32                        microcode;            /*   220     4 */

        /* size: 224, cachelines: 4, members: 30 */
        /* sum members: 219, holes: 2, sum holes: 5 */
        /* last cacheline: 32 bytes */
};

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [next] | [standalone]

#1580660

On 14 February 2017 at 17:17, Borislav Petkov <bp@alien8.de> wrote:
> On Sun, Feb 12, 2017 at 10:12:07PM +0100, Mathias Krause wrote:
>> Those member serve no purpose -- not even fill padding for alignment or
>> such. So just get rid of them.
>
> Well, almost. You need the wp_works_ok removal patch too, otherwise you
> have the 3 bytes hole below.

Heh, indeed! But only since commit 79a8b9aa388b ("x86/CPU/AMD: Bring
back Compute Unit ID") ;)

> But the wp_works_ok goes away too so I guess that's fine.
>
> $ pahole -C cpuinfo_x86 vmlinux
> struct cpuinfo_x86 {
>         __u8                       x86;                  /*     0     1 */
>         __u8                       x86_vendor;           /*     1     1 */
>         __u8                       x86_model;            /*     2     1 */
>         __u8                       x86_mask;             /*     3     1 */
>         char                       wp_works_ok;          /*     4     1 */
>         __u8                       x86_virt_bits;        /*     5     1 */
>         __u8                       x86_phys_bits;        /*     6     1 */
>         __u8                       x86_coreid_bits;      /*     7     1 */
>         __u8                       cu_id;                /*     8     1 */
>
>         /* XXX 3 bytes hole, try to pack */

The cu_id member is "new". Without it there would be no hole. But,
yeah, without wp_works_ok everything's fine again.

Cheers,
Mathias

[toc] | [prev] | [next] | [standalone]

#1580720

Hi Boris,

On Tue, Feb 14, 2017 at 5:17 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Sun, Feb 12, 2017 at 10:12:07PM +0100, Mathias Krause wrote:
>> Those member serve no purpose -- not even fill padding for alignment or
>> such. So just get rid of them.
>
> Well, almost. You need the wp_works_ok removal patch too, otherwise you
> have the 3 bytes hole below.

That's because you removed a char in commit 93a829e8e2c292f1
("x86, cpu: Convert FDIV bug detection), without compensating with padding ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1580745

On Tue, Feb 14, 2017 at 06:56:22PM +0100, Geert Uytterhoeven wrote:
> That's because

No, what Mathias said.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1580757

On 02/14/17 09:56, Geert Uytterhoeven wrote:
>>
>> Well, almost. You need the wp_works_ok removal patch too, otherwise you
>> have the 3 bytes hole below.
> 
> That's because you removed a char in commit 93a829e8e2c292f1
> ("x86, cpu: Convert FDIV bug detection), without compensating with padding ;-)
> 

Padding isn't a problem (other than efficiency) for a structure which is
strictly internal to the kernel as opposed to an ABI structure.

	-hpa

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web