Show multiple user PF's in the output#3
Show multiple user PF's in the output#3damiandcoherent wants to merge 1 commit intoDispatchCode:mainfrom
Conversation
damiandcoherent
force-pushed
the
main
branch
from
1db8670 to
44c54e4
Compare
| struct perf_branch_entry lbr[MAX_LBR_ENTRIES]; | ||
|
|
||
| u64 tai; // time atomic international | ||
| u64 cr2_tai[MAX_USER_PF_ENTRIES]; |
There was a problem hiding this comment.
Can we maybe group the CR2 stuff? E.g. via a field struct page_fault_info pf[MAX_USER_PF_ENTRIES] in event_t.
| printf("\"cr2_fault\":null"); | ||
| for (u32 i = 0; i < e->cr2_userpf_entry_count; i++) | ||
| { | ||
| printf("\"cr2_fault_%u\":\"0x%016llx\",", i, e->regs.cr2_faults[i]); |
There was a problem hiding this comment.
I think from a post-processing perspective, it might make more sense as
"page_faults": [
{
"tai": 123456,
"cr2": "0x123456",
"err": "0x123456"
},
...
]This can be done easily if we group the page fault info differently in event_t, see my comment in the header.
| }; | ||
|
|
||
| struct cr2_stat { | ||
| __u64 cr2; |
There was a problem hiding this comment.
Don't we also have a u64 typedef from vmlinux.h?
|
|
||
| if (++stats->count > MAX_USER_PF_ENTRIES) | ||
| stats->count = MAX_USER_PF_ENTRIES; | ||
| } |
There was a problem hiding this comment.
head >= MAX_USER_PF_ENTRIES should not be possible. Should we maybe trace this, and reset it to 0? (I guess this branch is meant for the verifier..?)
| stats->stat[stats->head] = *value; | ||
|
|
||
| if (++stats->head >= MAX_USER_PF_ENTRIES) | ||
| stats->head -= MAX_USER_PF_ENTRIES; |
There was a problem hiding this comment.
Why not reset it to 0? Concurrency??
| } | ||
| } | ||
|
|
||
| inline struct cr2_stat* cr2stats_get(struct cr2_stats* stats, u32 index) { |
There was a problem hiding this comment.
Might make sense to document that this "index" is not an index into the array, but into the ring buffer (meaning, the index 0 will get you the oldest element in the ring buffer).
|
|
||
| event->regs.cr2 = task->thread.cr2; | ||
| event->regs.cr2_fault = -1; | ||
| event->cr2_userpf_entry_count = 0; |
There was a problem hiding this comment.
I think we should move the page fault stuff out of the if (regs) branch. It also works if bpf_task_pt_regs failed.
| event->cr2_tai[i] = stat->tai; | ||
|
|
||
| ++event->cr2_userpf_entry_count; | ||
| } |
There was a problem hiding this comment.
Should we perhaps break the loop if stat == NULL?
| cr2 = ctx->address; | ||
| stat.cr2 = ctx->address; | ||
| stat.err = ctx->error_code; | ||
| stat.tai = bpf_ktime_get_tai_ns(); |
There was a problem hiding this comment.
Have you measured the performance impact when executing lots of page faults?
3 participants
No description provided.