From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Andrii Nakryiko <andriin@fb.com>
Cc: andrii.nakryiko@gmail.com, dwarves@vger.kernel.org,
bpf@vger.kernel.org, ast@fb.com, yhs@fb.com
Subject: Re: [PATCH pahole 0/3] fix handling of bitfields in btf_loader
Date: Fri, 22 Feb 2019 19:02:43 -0300 [thread overview]
Message-ID: <20190222220243.GI26132@kernel.org> (raw)
In-Reply-To: <20190220205732.2514975-1-andriin@fb.com>
Em Wed, Feb 20, 2019 at 12:57:29PM -0800, Andrii Nakryiko escreveu:
> This patchset fixes the logic of determining bitfield_offsets and
> initial bit_offset when using BTF type information. It eliminates all
> the remaining discrepancies, when doing btfdiff on vmlinux image, module
> two instances of incorrectly reporting struct member type name when
> bitfield is the very first field in a struct, which is only happening
> when using DWARF data.
>
> Patch #1 makes btfdiff script easier to use during local development.
>
> Patch #2 fixes list of known base type names to handle clang-generated
> type descriptions better.
>
> Patch #3 fixes bitfield handling logic in btf_loader.
Thanks a lot! Applied.
And here I see no differences in my vmlinux:
[acme@quaco pahole]$ btfdiff vmlinux
[acme@quaco pahole]$
[acme@quaco pahole]$ perf stat pahole -F dwarf --flat_arrays --show_private_classes vmlinux > /tmp/dwarf
Performance counter stats for 'pahole -F dwarf --flat_arrays --show_private_classes vmlinux':
36,140.58 msec task-clock:u # 0.998 CPUs utilized
0 context-switches:u # 0.000 K/sec
0 cpu-migrations:u # 0.000 K/sec
359,238 page-faults:u # 0.010 M/sec
83,741,861,962 cycles:u # 2.317 GHz
99,851,434,338 instructions:u # 1.19 insn per cycle
20,974,798,749 branches:u # 580.367 M/sec
288,410,045 branch-misses:u # 1.38% of all branches
36.230455825 seconds time elapsed
35.037146000 seconds user
0.906593000 seconds sys
[acme@quaco pahole]$ perf stat pahole -F btf vmlinux > /tmp/btf
Performance counter stats for 'pahole -F btf vmlinux':
215.21 msec task-clock:u # 0.994 CPUs utilized
0 context-switches:u # 0.000 K/sec
0 cpu-migrations:u # 0.000 K/sec
3,195 page-faults:u # 0.015 M/sec
460,363,235 cycles:u # 2.139 GHz
546,688,575 instructions:u # 1.19 insn per cycle
104,488,443 branches:u # 485.522 M/sec
2,012,815 branch-misses:u # 1.93% of all branches
0.216609739 seconds time elapsed
0.200431000 seconds user
0.014834000 seconds sys
[acme@quaco pahole]$
[acme@quaco pahole]$ wc -l /tmp/dwarf
106101 /tmp/dwarf
[acme@quaco pahole]$ wc -l /tmp/btf
106101 /tmp/btf
[acme@quaco pahole]$
[acme@quaco pahole]$ pahole -F btf -C sk_buff_head vmlinux
struct sk_buff_head {
struct sk_buff * next; /* 0 8 */
struct sk_buff * prev; /* 8 8 */
__u32 qlen; /* 16 4 */
spinlock_t lock; /* 20 4 */
/* size: 24, cachelines: 1, members: 4 */
/* last cacheline: 24 bytes */
};
[acme@quaco pahole]$ pahole --expand_types -F btf -C sk_buff_head vmlinux
struct sk_buff_head {
struct sk_buff * next; /* 0 8 */
struct sk_buff * prev; /* 8 8 */
/* typedef __u32 */ unsigned int qlen; /* 16 4 */
/* typedef spinlock_t */ struct spinlock {
union {
struct raw_spinlock {
/* typedef arch_spinlock_t */ struct qspinlock {
union {
/* typedef atomic_t */ struct {
int counter; /* 20 4 */
} val; /* 20 4 */
struct {
/* typedef u8 -> __u8 */ unsigned char locked; /* 20 1 */
/* typedef u8 -> __u8 */ unsigned char pending; /* 21 1 */
}; /* 20 2 */
struct {
/* typedef u16 -> __u16 */ short unsigned int locked_pending; /* 20 2 */
/* typedef u16 -> __u16 */ short unsigned int tail; /* 22 2 */
}; /* 20 4 */
}; /* 20 4 */
} raw_lock; /* 20 4 */
} rlock; /* 20 4 */
}; /* 20 4 */
} lock; /* 20 4 */
/* size: 24, cachelines: 1, members: 4 */
/* last cacheline: 24 bytes */
};
[acme@quaco pahole]$
The problem with the packed structs is gone:
[acme@quaco pahole]$ pahole -F dwarf -C screen_info vmlinux > /tmp/dwarf
[acme@quaco pahole]$ pahole -F btf -C screen_info vmlinux > /tmp/btf
[acme@quaco pahole]$ diff -u /tmp/btf /tmp/dwarf
[acme@quaco pahole]$ pahole --hex -F btf -C screen_info vmlinux
struct screen_info {
__u8 orig_x; /* 0 0x1 */
__u8 orig_y; /* 0x1 0x1 */
__u16 ext_mem_k; /* 0x2 0x2 */
__u16 orig_video_page; /* 0x4 0x2 */
__u8 orig_video_mode; /* 0x6 0x1 */
__u8 orig_video_cols; /* 0x7 0x1 */
__u8 flags; /* 0x8 0x1 */
__u8 unused2; /* 0x9 0x1 */
__u16 orig_video_ega_bx; /* 0xa 0x2 */
__u16 unused3; /* 0xc 0x2 */
__u8 orig_video_lines; /* 0xe 0x1 */
__u8 orig_video_isVGA; /* 0xf 0x1 */
__u16 orig_video_points; /* 0x10 0x2 */
__u16 lfb_width; /* 0x12 0x2 */
__u16 lfb_height; /* 0x14 0x2 */
__u16 lfb_depth; /* 0x16 0x2 */
__u32 lfb_base; /* 0x18 0x4 */
__u32 lfb_size; /* 0x1c 0x4 */
__u16 cl_magic; /* 0x20 0x2 */
__u16 cl_offset; /* 0x22 0x2 */
__u16 lfb_linelength; /* 0x24 0x2 */
__u8 red_size; /* 0x26 0x1 */
__u8 red_pos; /* 0x27 0x1 */
__u8 green_size; /* 0x28 0x1 */
__u8 green_pos; /* 0x29 0x1 */
__u8 blue_size; /* 0x2a 0x1 */
__u8 blue_pos; /* 0x2b 0x1 */
__u8 rsvd_size; /* 0x2c 0x1 */
__u8 rsvd_pos; /* 0x2d 0x1 */
__u16 vesapm_seg; /* 0x2e 0x2 */
__u16 vesapm_off; /* 0x30 0x2 */
__u16 pages; /* 0x32 0x2 */
__u16 vesa_attributes; /* 0x34 0x2 */
__u32 capabilities; /* 0x36 0x4 */
__u32 ext_lfb_base; /* 0x3a 0x4 */
__u8 _reserved[2]; /* 0x3e 0x2 */
/* size: 64, cachelines: 1, members: 36 */
};
[acme@quaco pahole]$
And the definition in include/uapi/linux/screen_info.h is:
/*
* These are set up by the setup-routine at boot-time:
*/
struct screen_info {
__u8 orig_x; /* 0x00 */
__u8 orig_y; /* 0x01 */
__u16 ext_mem_k; /* 0x02 */
__u16 orig_video_page; /* 0x04 */
__u8 orig_video_mode; /* 0x06 */
__u8 orig_video_cols; /* 0x07 */
__u8 flags; /* 0x08 */
__u8 unused2; /* 0x09 */
__u16 orig_video_ega_bx;/* 0x0a */
__u16 unused3; /* 0x0c */
__u8 orig_video_lines; /* 0x0e */
__u8 orig_video_isVGA; /* 0x0f */
__u16 orig_video_points;/* 0x10 */
/* VESA graphic mode -- linear frame buffer */
__u16 lfb_width; /* 0x12 */
__u16 lfb_height; /* 0x14 */
__u16 lfb_depth; /* 0x16 */
__u32 lfb_base; /* 0x18 */
__u32 lfb_size; /* 0x1c */
__u16 cl_magic, cl_offset; /* 0x20 */
__u16 lfb_linelength; /* 0x24 */
__u8 red_size; /* 0x26 */
__u8 red_pos; /* 0x27 */
__u8 green_size; /* 0x28 */
__u8 green_pos; /* 0x29 */
__u8 blue_size; /* 0x2a */
__u8 blue_pos; /* 0x2b */
__u8 rsvd_size; /* 0x2c */
__u8 rsvd_pos; /* 0x2d */
__u16 vesapm_seg; /* 0x2e */
__u16 vesapm_off; /* 0x30 */
__u16 pages; /* 0x32 */
__u16 vesa_attributes; /* 0x34 */
__u32 capabilities; /* 0x36 */
__u32 ext_lfb_base; /* 0x3a */
__u8 _reserved[2]; /* 0x3e */
} __attribute__((packed));
Which ones where these: "module two instances of incorrectly reporting struct
member type name when bitfield is the very first field in a struct, which is
only happening when using DWARF data." ?
- Arnaldo
next prev parent reply other threads:[~2019-02-22 22:02 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-20 20:57 [PATCH pahole 0/3] fix handling of bitfields in btf_loader Andrii Nakryiko
2019-02-20 20:57 ` [PATCH pahole 1/3] btfdiff: support specifying custom pahole location Andrii Nakryiko
2019-02-20 20:57 ` [PATCH pahole 2/3] pahole: complete list of base type names Andrii Nakryiko
2019-02-20 20:57 ` [PATCH pahole 3/3] btf_loader: fix bitfield fixup code Andrii Nakryiko
2019-02-22 22:02 ` Arnaldo Carvalho de Melo [this message]
2019-02-22 23:23 ` [PATCH pahole 0/3] fix handling of bitfields in btf_loader Andrii Nakryiko
2019-02-25 16:02 ` Arnaldo Carvalho de Melo
2019-02-25 18:59 ` Arnaldo Carvalho de Melo
2019-02-25 19:42 ` Andrii Nakryiko
2019-02-25 20:08 ` encoding BTF on glibc was: " Arnaldo Carvalho de Melo
2019-02-25 20:51 ` Andrii Nakryiko
2019-02-25 21:36 ` Andrii Nakryiko
2019-02-26 0:34 ` Andrii Nakryiko
2019-02-26 5:37 ` Yonghong Song
2019-02-26 5:44 ` Alexei Starovoitov
2019-02-26 5:45 ` Andrii Nakryiko
2019-02-26 6:03 ` Yonghong Song
2019-02-26 17:22 ` Andrii Nakryiko
2019-02-26 17:24 ` Yonghong Song
2019-02-26 17:34 ` Andrii Nakryiko
2019-02-26 13:11 ` Arnaldo Carvalho de Melo
2019-02-26 17:14 ` Andrii Nakryiko
2019-02-26 17:45 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190222220243.GI26132@kernel.org \
--to=arnaldo.melo@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=dwarves@vger.kernel.org \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.