-
Notifications
You must be signed in to change notification settings - Fork 10
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
Readme updates #302
Readme updates #302
Conversation
Fixes #247 |
|
||
You should take some time to review the services running on your benchmarking | ||
machine. Debian especially has a habit of starting daemons which get pulled in | ||
by dependencies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sort of advice is good to have!
On Linux, list services with: | ||
|
||
``` | ||
# systemctl | grep running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we used $
for the system prompt below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A hash is used to show it has to be run by root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I think this is the only place we used a hash prompt, and where we used sudo
, we didn't have a prompt at all!
'aperf_counts': {...} # Per-core APERF deltas | ||
# (structure same as 'core_cycle_counts') | ||
'mperf_counts': {...} # Per-core MPERF deltas | ||
# (structure same as 'core_cycle_counts') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth saying here, or somewhere else, that aperf/mperf
needs to be computed to make sense of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably somewhere else, but that place doesn't exist. It's true that Krun docs are not complete, but I don't think now is the time to launch into that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
Minor comments, but this looks good |
Assuming you are happy with my responses, OK to squash? |
Can you just look at line 122 and make it like your other root prompts? It should be:
not:
to match the changes you want to merge here. |
Sure, fixed. |
Great, let's squash then! |
ca27222
to
b84bbb9
Compare
splat. |
Update JSON structure, and talk a bit about auditing system services.