-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[sonic-bootchart] add document on integrating systemd-bootchart #1001
[sonic-bootchart] add document on integrating systemd-bootchart #1001
Conversation
stepanblyschak
commented
May 16, 2022
•
edited
Loading
edited
PR title | state | context |
---|---|---|
[sonic_debian_extension] install systemd-bootchart | ||
[sonic-bootchart] add sonic-bootchart |
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
doc/profiling/sonic_bootchart.md
Outdated
Update configuration example: | ||
|
||
``` | ||
admin@sonic:~$ sudo sonic-bootchart config --samples 500 --frequency 10 |
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.
Understand bootchart's parameters are sample and frequency, as cli wrapper, I suggest:
- Instead of sample counts, use time span as parameter, wrapper calculate the sample count = time span x frequency
- having meaningful default value for time span and frequency.
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.
- Changed to time span
- The defaults are mentioned in this section - https://github.com/stepanblyschak/SONiC/blob/sonic-bootchart/doc/profiling/sonic_bootchart.md#high-level-design
|
||
N/A | ||
|
||
### Open/Action items |
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.
Suggest adding code at initramfs stage to generate bootchart configuration file according to kernel parameters. So that we could have bootchart configured for upcoming boot generated.
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.
Instead of initramfs we may add this to sonic-installer (where we usually perform config/packages migration tasks).
Added an open action.
Did you evaluate the script Also, please wrap lines after 80(?) characters in the Markdown documents, so it’s easier to comment on specific parts of a paragraph in GitHub. |
@paulmenzel No, this tool wasn't considered. If needed, one may change the tool used while still have the same SONiC CLI command. |
@stepanblyschak please update the PR description with the list of PRs to get into as part of the feature. |
@liat-grozovik Updated description with PR list |
- Why I did it Implemented sonic-net/SONiC#1001 - How I did it Install systemd-bootchart tool and provide default config for it. - How to verify it Run build and verify systemd-bootchart is installed. Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
- What I did Implemented sonic-net/SONiC#1001 - How I did it Added a new sonic-bootchart script and added UT for it - How to verify it Run on the switch. Depends on sonic-net/sonic-buildimage#11047 Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
- What I did Implemented sonic-net/SONiC#1001 - How I did it Added a new sonic-bootchart script and added UT for it - How to verify it Run on the switch. Depends on sonic-net/sonic-buildimage#11047 Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>