There was an additional 7 bytes of compiler-inserted padding at the
end of the structure visible via 'ptype /o' in gdb.
Obtained from: CheriBSD
Details
Details
- Reviewers
br mhorne jrtc27 - Commits
- rS364176: Correct padding length for RISC-V PCPU data.
- built a kernel and compared ptype /o before/after
Diff Detail
Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 32762 Build 30197: arc lint + arc unit
Event Timeline
Comment Actions
Before:
(gdb) ptype /o struct pcpu
/* offset | size */ type = struct pcpu {
/* 0 | 8 */ struct thread *pc_curthread;
/* 8 | 8 */ struct thread *pc_idlethread;
/* 16 | 8 */ struct thread *pc_fpcurthread;
/* 24 | 8 */ struct thread *pc_deadthread;
/* 32 | 8 */ struct pcb *pc_curpcb;
/* 40 | 16 */ void *pc_sched;
/* 48 | 8 */ uint64_t pc_switchtime;
/* 56 | 4 */ int pc_switchticks;
/* 60 | 4 */ u_int pc_cpuid;
/* 64 | 8 */ struct {
/* 64 | 8 */ struct pcpu *stqe_next;
/* total size (bytes): 8 */
} pc_allcpu;
/* 72 | 8 */ struct lock_list_entry *pc_spinlocks;
/* 80 | 40 */ long pc_cp_time[5];
/* 120 | 8 */ struct device *pc_device;
/* 128 | 16 */ void *pc_netisr;
/* 136 | 4 */ int pc_unused1;
/* 140 | 4 */ int pc_domain;
/* 144 | 16 */ struct rm_queue {
/* 144 | 8 */ struct rm_queue * volatile rmq_next;
--Type <RET> for more, q to quit, c to continue without paging--
/* 152 | 8 */ struct rm_queue * volatile rmq_prev;
/* total size (bytes): 16 */
} pc_rm_queue;
/* 160 | 8 */ uintptr_t pc_dynamic;
/* 168 | 8 */ uint64_t pc_early_dummy_counter;
/* 176 | 8 */ uintptr_t pc_zpcpu_offset;
/* 184 | 8 */ struct pmap *pc_curpmap;
/* 192 | 4 */ uint32_t pc_pending_ipis;
/* 196 | 4 */ uint32_t pc_hart;
/* 200 | 49 */ char __pad[49];
/* XXX 7-byte padding */
/* total size (bytes): 256 */
}after:
(gdb) ptype /o struct pcpu
/* offset | size */ type = struct pcpu {
/* 0 | 8 */ struct thread *pc_curthread;
/* 8 | 8 */ struct thread *pc_idlethread;
/* 16 | 8 */ struct thread *pc_fpcurthread;
/* 24 | 8 */ struct thread *pc_deadthread;
/* 32 | 8 */ struct pcb *pc_curpcb;
/* 40 | 8 */ void *pc_sched;
/* 48 | 8 */ uint64_t pc_switchtime;
/* 56 | 4 */ int pc_switchticks;
/* 60 | 4 */ u_int pc_cpuid;
/* 64 | 8 */ struct {
/* 64 | 8 */ struct pcpu *stqe_next;
/* total size (bytes): 8 */
} pc_allcpu;
/* 72 | 8 */ struct lock_list_entry *pc_spinlocks;
/* 80 | 40 */ long pc_cp_time[5];
/* 120 | 8 */ struct device *pc_device;
/* 128 | 8 */ void *pc_netisr;
/* 136 | 4 */ int pc_unused1;
/* 140 | 4 */ int pc_domain;
/* 144 | 16 */ struct rm_queue {
/* 144 | 8 */ struct rm_queue * volatile rmq_next;
--Type <RET> for more, q to quit, c to continue without paging--
/* 152 | 8 */ struct rm_queue * volatile rmq_prev;
/* total size (bytes): 16 */
} pc_rm_queue;
/* 160 | 8 */ uintptr_t pc_dynamic;
/* 168 | 8 */ uint64_t pc_early_dummy_counter;
/* 176 | 8 */ uintptr_t pc_zpcpu_offset;
/* 184 | 8 */ struct pmap *pc_curpmap;
/* 192 | 4 */ uint32_t pc_pending_ipis;
/* 196 | 4 */ uint32_t pc_hart;
/* 200 | 56 */ char __pad[56];
/* total size (bytes): 256 */
}Comment Actions
- Can we please properly document where this magic value comes from?
- Can we make pcpu_aux.h also assert that offsetof(__pad) + sizeof(__pad) == sizeof(struct pcpu) so all the padding is accounted for?
| sys/riscv/include/pcpu.h | ||
|---|---|---|
| 51 | i.e. something like this is probably enough for explaining why this exists and where the value comes from | |
| sys/riscv/include/pcpu.h | ||
|---|---|---|
| 51 | Ah, it's not even that: sys/riscv/include/pcpu_aux.h:_Static_assert(PAGE_SIZE % sizeof(struct pcpu) == 0, "fix pcpu size"); It's checking that they are a _factor_ of PAGE_SIZE, presumably so we can guarantee that only one page needs to be mapped for each PCPU (i.e. not just that it fits in a page, but also that it never straddles two pages)? (except for amd64/i386, which do sizeof(struct pcpu) == UMA_PCPU_ALLOC_SIZE, where the latter is just defined to PAGE_SIZE in sys/sys/pcpu.h) | |
| sys/riscv/include/pcpu_aux.h | ||
|---|---|---|
| 50 | Neat, I hadn't considered that you could get the size of a struct member that way. | |