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.

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

[finalize-warmboot.sh] reset cpufreq governor to default #19634

Merged
merged 4 commits into from
Nov 4, 2024

Conversation

stepanblyschak
Copy link
Collaborator

@stepanblyschak stepanblyschak commented Jul 19, 2024

Why I did it

Set cpufreq.default_governor to performance for faster boot time. We observe consistent 1 sec improvement across several devices.

The change in finalize-warmboot.sh restores the default governor after fast or warm boot is finished.

NOTE: This will apply to upgrades starting from 202405 since this is set in shutdown path to avoid any extra scripts running at boot time. Upgrade from older versions/branches will require a runtime patch to fast-reboot and warm-reboot script.

Work item tracking
  • Microsoft ADO (number only):

How I did it

After fast or warm boot is finished restore to default governor.

How to verify it

Run fast-reboot or warm-reboot. Check:

admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
performance

After boot is finalized check that it is reset back to default:

admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
schedutil

Tested with sonic-net/sonic-utilities#3435

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305
  • 202311
  • 202405

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@bingwang-ms
Copy link
Contributor

@saiarcot895 @vaibhavhd Can you please help review?

@liat-grozovik
Copy link
Collaborator

@vaibhavhd kindly reminder for code review feedback/approval.

@vaibhavhd
Copy link
Contributor

Comments are open on utilities PR.

@@ -101,16 +101,30 @@ function check_list()
echo ${RET_LIST}
}

function set_cpufreq_governor() {
local -r governor="$1"
echo "$governor" | tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor 1> /dev/null \
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this setting (or the way we are writing to a file) be effective at runtime?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes


function finalize_common() {
# Read default governor from kernel config
local -r default_governor=$(cat "/boot/config-$(uname -r)" | grep -o 'CONFIG_CPU_FREQ_DEFAULT_GOV_[^=]*=y')
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the guarantee that "/boot/config-$(uname -r)" will have the default governor setting?

Do you need to sanity check on the value and type of default_governor here? What if the cat or grep result in a failure? Wouldn't the code proceed to assign invalid value to /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's part of our sonic linux kernel config, why would we remove it or set it to invalid value?

function finalize_common() {
# Read default governor from kernel config
local -r default_governor=$(cat "/boot/config-$(uname -r)" | grep -o 'CONFIG_CPU_FREQ_DEFAULT_GOV_[^=]*=y')
set_cpufreq_governor "$default_governor"
Copy link
Contributor

Choose a reason for hiding this comment

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

The utilities change is made for MLNX platforms only. I think it would be best to make this call only for MLNX platforms as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @vaibhavhd I applied your suggestion. While testing that modification found that the | grep is incorrect causing governor to not reset to default. That bug was introduced in "Handle review comments" commit, don't remember why I did it, but I reverted to previous default governor parsing method.

Copy link
Contributor

@vaibhavhd vaibhavhd left a comment

Choose a reason for hiding this comment

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

Approved with suggestions.

Signed-off-by: Stepan Blyschak <[email protected]>
Signed-off-by: Stepan Blyschak <[email protected]>
@stepanblyschak
Copy link
Collaborator Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dgsudharsan
Copy link
Collaborator

@qiluo-msft can you please help merge?

@qiluo-msft qiluo-msft merged commit 33000a5 into sonic-net:master Nov 4, 2024
23 checks passed
rkavitha-hcl pushed a commit to rkavitha-hcl/sonic-buildimage that referenced this pull request Nov 15, 2024
…9634)

#### Why I did it

Set cpufreq.default_governor to *performance* for faster boot time. We observe consistent 1 sec improvement across several devices.

The change in finalize-warmboot.sh restores the default governor after fast or warm boot is finished.

**NOTE**: This will apply to upgrades starting from 202405 since this is set in shutdown path to avoid any extra scripts running at boot time. Upgrade from older versions/branches will require a runtime patch to fast-reboot and warm-reboot script.

#### How I did it

After fast or warm boot is finished restore to default governor.

#### How to verify it

Run fast-reboot or warm-reboot. Check:
```
admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
performance
```

After boot is finalized check that it is reset back to default:
```
admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
schedutil
```

Tested with sonic-net/sonic-utilities#3435
aidan-gallagher pushed a commit to aidan-gallagher/sonic-buildimage that referenced this pull request Nov 16, 2024
…9634)

#### Why I did it

Set cpufreq.default_governor to *performance* for faster boot time. We observe consistent 1 sec improvement across several devices.

The change in finalize-warmboot.sh restores the default governor after fast or warm boot is finished.

**NOTE**: This will apply to upgrades starting from 202405 since this is set in shutdown path to avoid any extra scripts running at boot time. Upgrade from older versions/branches will require a runtime patch to fast-reboot and warm-reboot script.

#### How I did it

After fast or warm boot is finished restore to default governor.

#### How to verify it

Run fast-reboot or warm-reboot. Check:
```
admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
performance
```

After boot is finalized check that it is reset back to default:
```
admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
schedutil
```

Tested with sonic-net/sonic-utilities#3435
@bingwang-ms
Copy link
Contributor

@stepanblyschak Can you raise a PR to 202405 to do the cherry-pick? The auto cherry-pick is not working for this PR.

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Dec 4, 2024
…9634)

#### Why I did it

Set cpufreq.default_governor to *performance* for faster boot time. We observe consistent 1 sec improvement across several devices.

The change in finalize-warmboot.sh restores the default governor after fast or warm boot is finished.

**NOTE**: This will apply to upgrades starting from 202405 since this is set in shutdown path to avoid any extra scripts running at boot time. Upgrade from older versions/branches will require a runtime patch to fast-reboot and warm-reboot script.

#### How I did it

After fast or warm boot is finished restore to default governor.

#### How to verify it

Run fast-reboot or warm-reboot. Check:
```
admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
performance
```

After boot is finalized check that it is reset back to default:
```
admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
schedutil
```

Tested with sonic-net/sonic-utilities#3435
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #21030

mssonicbld pushed a commit that referenced this pull request Dec 5, 2024
#### Why I did it

Set cpufreq.default_governor to *performance* for faster boot time. We observe consistent 1 sec improvement across several devices.

The change in finalize-warmboot.sh restores the default governor after fast or warm boot is finished.

**NOTE**: This will apply to upgrades starting from 202405 since this is set in shutdown path to avoid any extra scripts running at boot time. Upgrade from older versions/branches will require a runtime patch to fast-reboot and warm-reboot script.

#### How I did it

After fast or warm boot is finished restore to default governor.

#### How to verify it

Run fast-reboot or warm-reboot. Check:
```
admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
performance
```

After boot is finalized check that it is reset back to default:
```
admin@sonic:~$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
schedutil
```

Tested with sonic-net/sonic-utilities#3435
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants