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.

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

Conversation

pingerino
Copy link
Contributor

Fix errors on 64-bit.

@pingerino
Copy link
Contributor Author

Signed CLA has been sent. This looks good to merge history wise, just let me know if you want any changes.

@kent-mcleod kent-mcleod self-assigned this Jul 15, 2019
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

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

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...

Fix errors on 64-bit.

Signed-off-by Gerwin Klein <[email protected]>
@lsf37 lsf37 self-assigned this Sep 9, 2021
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

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants