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.

Sign up for GitHub

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

[fast/warm-reboot] add cpufreq.default_governor=performance to BOOT_OPTIONS #3435

Merged
merged 2 commits into from
Oct 14, 2024
Merged

[fast/warm-reboot] add cpufreq.default_governor=performance to BOOT_OPTIONS #3435

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30a4278
[fast/warm-reboot] add cpufreq.default_governor=performance to BOOT_O…
stepanblyschak Jul 18, 2024
d2daf60
set governor only for nvidia
stepanblyschak Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions scripts/fast-reboot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,10 @@ function setup_reboot_variables()
local fstype=$(blkid -o value -s TYPE ${sonic_dev})
BOOT_OPTIONS="${BOOT_OPTIONS} ssd-upgrader-part=${sonic_dev},${fstype}"
fi

# Set governor to performance to speed up boot process.
# The governor is reset back to kernel default in warmboot-finalizer script.
BOOT_OPTIONS="${BOOT_OPTIONS} cpufreq.default_governor=performance"
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest that you don't add this as a default behavior. This needs to be set by an option provided to warm/fast-reboot script.

That way we can tightly control when/if this is used.

Copy link
Contributor Author

@stepanblyschak stepanblyschak Aug 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@vaibhavhd

I don't think an optimization is an optinal feature.

We already did enhancements which are enabled by default, e.g. LACP retry count, that even had a larger impact as the changes are applied also on the peer side.

Also, from the testing perspective we would need to test all combinations of possible optimization flags user may pass to the script.

From the user perspective, it becomes unclear what flags he needs to use, what is heavily tested and what is not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need an architecture check? Is this supported on armhf and arm64 as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

This may be useful in some scenarios and I think user should be given an option to control this behavior. Some examples I can think of are:

  1. High CPU usage during boot up path.
  2. High thermal load due to use new file location for sys eeprom and sfp eeprom on msn2700 #1.
  3. If this mode has dependency on underlying hardware then it depends on the hardware if this is supported.

Mostly these concerns are not critical on new platforms. But some old/slow platforms do not really want this high CPU, high fan activity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vaibhavhd @saiarcot895 I limit this setting for nvidia where this is tested and working.

}

function check_docker_exec()
Expand Down
Loading