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

Columns for delaystats #665

Closed
brendangregg opened this Issue Aug 2, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@brendangregg

brendangregg commented Aug 2, 2017

Please add column for delaystats to htop. They've been in Linux for a long time, and show delays for different types of resources. There's not many tools that use them, so far.

The Linux kernel source ships with an example: https://github.com/torvalds/linux/blob/master/tools/accounting/getdelays.c

tools/accounting> ./getdelays -d -p $$
print delayacct stats ON
PID	4961


CPU             count     real total  virtual total    delay total  delay average
                  367      160000000      150819518        1942403          0.005ms
IO              count    delay total  delay average
                   40      306740944              7ms
SWAP            count    delay total  delay average
                    0              0              0ms
RECLAIM         count    delay total  delay average
                    0              0              0ms

It's the delay totals that are interesting. They are measured in nanoseconds, but presenting them in htop I think it would make more sense as percentages. Eg, to show 50% CPU delays for a process (thread) would mean it's spending half it's time waiting for a turn on CPU.

@andrestc

This comment has been minimized.

Show comment
Hide comment
@andrestc

andrestc Aug 2, 2017

Contributor

Is this a good starting issue for someone who is new to the project(does not know the codebase)? If so, I would like to try and tackle this.

Contributor

andrestc commented Aug 2, 2017

Is this a good starting issue for someone who is new to the project(does not know the codebase)? If so, I would like to try and tackle this.

@brendangregg

This comment has been minimized.

Show comment
Hide comment
@brendangregg

brendangregg Aug 3, 2017

@andrestc it's a moderate change. A trivial addition would be a field that comes from a /proc file that htop is already reading. A bit harder would be adding a new /proc file that wasn't read before.

In this case, delaystats (sorry, I should be using the real name, which is "delay accounting") is read via the netlink interface, which is completely different, and more difficult, than a /proc reader. htop currently doesn't have a netlink reader (AFAIK). Fortunately, getdelays.c in the Linux source shows what needs to be done, but yes, it means adding a whole new data source to htop, which is why I'd call it moderate.

Is it good for a beginner? I'm not sure -- it would at least need a determined beginner who understood it's not, say, a 5 line trivial change. It'd probably end up a few hundred lines of code. Most of which may come from getdelays.c.

brendangregg commented Aug 3, 2017

@andrestc it's a moderate change. A trivial addition would be a field that comes from a /proc file that htop is already reading. A bit harder would be adding a new /proc file that wasn't read before.

In this case, delaystats (sorry, I should be using the real name, which is "delay accounting") is read via the netlink interface, which is completely different, and more difficult, than a /proc reader. htop currently doesn't have a netlink reader (AFAIK). Fortunately, getdelays.c in the Linux source shows what needs to be done, but yes, it means adding a whole new data source to htop, which is why I'd call it moderate.

Is it good for a beginner? I'm not sure -- it would at least need a determined beginner who understood it's not, say, a 5 line trivial change. It'd probably end up a few hundred lines of code. Most of which may come from getdelays.c.

@andrestc

This comment has been minimized.

Show comment
Hide comment
@andrestc

andrestc Aug 3, 2017

Contributor

@brendangregg thanks for the detailed answer.

I will give this a shot. But, as I might take slightly longer than someone with more experience on htop and its implementation details and I don't want to slow down anything, anyone can feel free to do it and beat me to it

Contributor

andrestc commented Aug 3, 2017

@brendangregg thanks for the detailed answer.

I will give this a shot. But, as I might take slightly longer than someone with more experience on htop and its implementation details and I don't want to slow down anything, anyone can feel free to do it and beat me to it

@hishamhm

This comment has been minimized.

Show comment
Hide comment
@hishamhm

hishamhm Aug 7, 2017

Owner

@andrestc Yes, that would be an excellent way to get started contributing to htop! @brendangregg's assessment is fully on point. It's not a quick patch, but it should be pretty self-contained, consisting basically of additions rather than modifications to the code base.

I won't be able to work on it myself over the next few weeks, so thank you for the initiative! Feel free to tag me via Github mentions if you have any questions about your work-in-progress.

Owner

hishamhm commented Aug 7, 2017

@andrestc Yes, that would be an excellent way to get started contributing to htop! @brendangregg's assessment is fully on point. It's not a quick patch, but it should be pretty self-contained, consisting basically of additions rather than modifications to the code base.

I won't be able to work on it myself over the next few weeks, so thank you for the initiative! Feel free to tag me via Github mentions if you have any questions about your work-in-progress.

@andrestc

This comment has been minimized.

Show comment
Hide comment
@andrestc

andrestc Aug 8, 2017

Contributor

Great, @hishamhm. I will probably be able to get some code done on the next weekend.

In the meantime, getdelays.c and others, e.g, the man for libnetlink, recommend the use of higher level libraries like libmnl or libnl. Should I aim to use one of these libraries and add such dependency to htop or just approach it like getdelays.c and just handle the parsing and etc myself?

Contributor

andrestc commented Aug 8, 2017

Great, @hishamhm. I will probably be able to get some code done on the next weekend.

In the meantime, getdelays.c and others, e.g, the man for libnetlink, recommend the use of higher level libraries like libmnl or libnl. Should I aim to use one of these libraries and add such dependency to htop or just approach it like getdelays.c and just handle the parsing and etc myself?

@hishamhm

This comment has been minimized.

Show comment
Hide comment
@hishamhm

hishamhm Aug 8, 2017

Owner

You can add the dependency, as that means less code to be added to htop itself. You can look at how hwloc was added as an optional dependency and follow that (adding the check/flag to configure.ac and then guarding the code with appropriate #defines).

Owner

hishamhm commented Aug 8, 2017

You can add the dependency, as that means less code to be added to htop itself. You can look at how hwloc was added as an optional dependency and follow that (adding the check/flag to configure.ac and then guarding the code with appropriate #defines).

@andrestc andrestc referenced this issue Aug 13, 2017

Merged

[WIP] Adds support for linux delay accounting #667

4 of 4 tasks complete
@hishamhm

This comment has been minimized.

Show comment
Hide comment
@hishamhm

hishamhm Dec 4, 2017

Owner

Linux delay stats are now merged! Major kudos to @andrestc for the implementation and thanks @brendangregg for the suggestion. A new release of htop with the feature is coming soon! :)

Owner

hishamhm commented Dec 4, 2017

Linux delay stats are now merged! Major kudos to @andrestc for the implementation and thanks @brendangregg for the suggestion. A new release of htop with the feature is coming soon! :)

@hishamhm hishamhm closed this Dec 4, 2017

@pmucci

This comment has been minimized.

Show comment
Hide comment
@pmucci

pmucci Dec 15, 2017

HI folks, it's worth noting in the documentation that delay stats are only available to root users. Even if you own the process, it seems delay accounting is only available to the root user or those with CAP_SYS_ADMIN capability.

pmucci commented Dec 15, 2017

HI folks, it's worth noting in the documentation that delay stats are only available to root users. Even if you own the process, it seems delay accounting is only available to the root user or those with CAP_SYS_ADMIN capability.

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