Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sel4utils: update benchmark_track.h #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+16 −16
Open

sel4utils: update benchmark_track.h #15

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions libsel4utils/include/sel4utils/benchmark_track.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ static inline void seL4_BenchmarkTrackDumpSummary(benchmark_track_kernel_entry_t
index++;
}

fprintf(fd, "Number of system call invocations %d\n", syscall_entries);
fprintf(fd, "Number of interrupt invocations %d\n", interrupt_entries);
fprintf(fd, "Number of user-level faults %d\n", userlevelfault_entries);
fprintf(fd, "Number of VM faults %d\n", vmfault_entries);
fprintf(fd, "Number of system call invocations %lu\n", (unsigned long) syscall_entries);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have SEL4_PRIu_word for this now

Copy link
Member

@lsf37 lsf37 Sep 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to update this, but I want to make sure I understand what I'm doing first :). If I traced this correctly, syscall_entries is seL4_Word, which should already be unsigned long. Am I missing something?

I.e. would simply

fprintf(fd, "Number of system call invocations %"SEL4_PRIu_word"\n", syscall_entries);

be correct now? (for both 32 and 64)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Member

@axel-h axel-h Sep 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, they all seem to be seL4_Word, so thy all should use SEL4_PRIu_word. And using "bla bla %15"SEL4_PRIu_word" bla bla" will also work. The issue here is that it would be 7 on 32-bit and 15 on 64-bit, but currently we have no constant we can use here. I've ran into this on another place also recently and thinking about a way to resolve this. The current way is always using %15"SEL4_PRIu_word" then. We could add SEL4_PRIzpu_word or SEL4_PRIu_word_zp to zero-prefix it...

fprintf(fd, "Number of interrupt invocations %lu\n", (unsigned long) interrupt_entries);
fprintf(fd, "Number of user-level faults %lu\n", (unsigned long) userlevelfault_entries);
fprintf(fd, "Number of VM faults %lu\n", (unsigned long) vmfault_entries);
}

/* Print out logged system call invocations */
Expand All @@ -62,14 +62,14 @@ static inline void seL4_BenchmarkTrackDumpFullSyscallLog(benchmark_track_kernel_

while (logBuffer[index].start_time != 0 && (index * sizeof(benchmark_track_kernel_entry_t)) < logSize) {
if (logBuffer[index].entry.path == Entry_Syscall) {
fprintf(fd, "| %-15d| %-15d| %-15llu| %-15d| %-15d| %-15d| %-15d|\n",
index,
logBuffer[index].entry.syscall_no,
(uint64_t) logBuffer[index].start_time,
logBuffer[index].duration,
logBuffer[index].entry.cap_type,
logBuffer[index].entry.invocation_tag,
logBuffer[index].entry.is_fastpath);
fprintf(fd, "| %-15lu| %-15lu| %-15"PRIu64"| %-15lu| %-15lu| %-15lu| %-15lu|\n",
Copy link
Member

@axel-h axel-h Nov 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is "*" in the printf() format args that we could use here to make this generic:

Suggested change
fprintf(fd, "| %-15lu| %-15lu| %-15"PRIu64"| %-15lu| %-15lu| %-15lu| %-15lu|\n",
#define WORD_INDENT ((CONFIG_WORD_SIZE / 4) - 1)
fprintf(fd, "| %-*"SEL4_PRIu_word"| %-*"SEL4_PRIu_word"| %-*"PRIu64"| %-*"SEL4_PRIu_word"| %-*"SEL4_PRIu_word"| %-*"SEL4_PRIu_word"| %-*"SEL4_PRIu_word"|\n",
WORD_INDENT, index,
WORD_INDENT, logBuffer[index].entry.syscall_no,
WORD_INDENT, logBuffer[index].start_time,
WORD_INDENT, logBuffer[index].duration,
WORD_INDENT, logBuffer[index].entry.cap_type,
WORD_INDENT, logBuffer[index].entry.invocation_tag,
WORD_INDENT, logBuffer[index].entry.is_fastpath);

(unsigned long) index,
(unsigned long) logBuffer[index].entry.syscall_no,
logBuffer[index].start_time,
(unsigned long) logBuffer[index].duration,
(unsigned long) logBuffer[index].entry.cap_type,
(unsigned long) logBuffer[index].entry.invocation_tag,
(unsigned long) logBuffer[index].entry.is_fastpath);
}
index++;
}
Expand All @@ -91,11 +91,11 @@ static inline void seL4_BenchmarkTrackDumpFullInterruptLog(benchmark_track_kerne

while (logBuffer[index].start_time != 0 && (index * sizeof(benchmark_track_kernel_entry_t)) < logSize) {
if (logBuffer[index].entry.path == Entry_Interrupt) {
fprintf(fd, "| %-15d| %-15d| %-15llu| %-15d|\n", \
index,
logBuffer[index].entry.word,
fprintf(fd, "| %-15lu| %-15lu| %-15"PRIu64"| %-15lu|\n", \
(unsigned long) index,
(unsigned long) logBuffer[index].entry.word,
logBuffer[index].start_time,
logBuffer[index].duration);
(unsigned long) logBuffer[index].duration);
}
index++;
}
Expand Down