Skip to content

Switch on schedstat support where needed.#16

Merged
sofar merged 1 commit intosystemd:masterfrom
sofar:pr2
Sep 14, 2016
Merged

Switch on schedstat support where needed.#16
sofar merged 1 commit intosystemd:masterfrom
sofar:pr2

Conversation

@sofar
Copy link
Copy Markdown
Contributor

@sofar sofar commented Aug 30, 2016

Newer kernels have schedstat disabled by default unless turned on
at runtime or through a kernel parameter/syscall. We should turn
this on irregardless if we can.

If the kernel is too old, any errors in this code will be ignored
as schedstat should be enabled already.

Fixes #12.

Comment thread src/bootchart.c Outdated
(void) setrlimit(RLIMIT_NOFILE, &rlim);

schfd = open("/proc/sys/kernel/sched_schedstats", O_WRONLY);
if (schfd)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if (schfd >= 0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't it better to close this file immediately? Keeping the fd open until the end of main seems a bit wasteful…

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we're probably fine not using autoclose here, I'll update this to close right away.

@sofar
Copy link
Copy Markdown
Contributor Author

sofar commented Sep 1, 2016

Updated with both comments addressed.

Comment thread src/bootchart.c
schfd = open("/proc/sys/kernel/sched_schedstats", O_WRONLY);
if (schfd >= 0)
write(schfd, "1\n", 2);
close(schfd);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please don't hate me ;)

Can you move the close underneath the if too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd hate it if you hadn't said that, cuz it's obviously not nice like this. :)

Newer kernels have schedstat disabled by default unless turned on
at runtime or through a kernel parameter/syscall. We should turn
this on irregardless if we can.

If the kernel is too old, any errors in this code will be ignored
as schedstat should be enabled already.

Fixes #12.
@sofar sofar merged commit 997273a into systemd:master Sep 14, 2016
@sofar
Copy link
Copy Markdown
Contributor Author

sofar commented Sep 14, 2016

a7478a4b09867a0bee999fe41ad0a95387839444

sofar added a commit that referenced this pull request Sep 14, 2016
Newer kernels have schedstat disabled by default unless turned on
at runtime or through a kernel parameter/syscall. We should turn
this on irregardless if we can.

If the kernel is too old, any errors in this code will be ignored
as schedstat should be enabled already.

Fixes #12.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants