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

Use tsc as clocksource for all x86 instances #2144

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mselim00
Copy link

@mselim00 mselim00 commented Feb 7, 2025

Issue #, if available:
Implements the recommendation by https://repost.aws/knowledge-center/manage-ec2-linux-clock-source to use tsc as the clocksource for all Intel/AMD instances.

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Testing Done

See this guide for recommended testing for PRs. Some tests may not apply. Completing tests and providing additional validation steps are not required, but it is recommended and may reduce review time and time to merge.

Tested on t4g.small, m6i.large, and c4.large instances to confirm clocksource across the 3 different cases.
t4g.small is an arm64 instance and showed the clocksource as arch_sys_counter
m6i.large is nitro based and showed as tsc.
c4.large is xen and showed as tsc.

@@ -27,15 +27,7 @@ function try-set-clocksource() {
fi
}

case "$(imds /latest/meta-data/system)" in
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove this script and the associated systemd unit altogether. On AL2, we add an arg to the kernel cmdline that makes tsc the default (if it's available).

We do need to check AL2023, I'm not sure if tsc is baked-in as the default there.

Copy link
Author

Choose a reason for hiding this comment

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

Ah ok, will look into this

@mselim00
Copy link
Author

mselim00 commented Feb 7, 2025

Couldn't find docs on it but anecdotally it seems AL2023 also defaults to Xen as the clocksource for instances with a Xen hypervisor. I've moved the grubby call out from the AL2 dedicated script to a shared utility for the two distros

@mselim00 mselim00 force-pushed the correct-clocksource branch 3 times, most recently from a522237 to c72e3f2 Compare February 7, 2025 06:04
@mselim00 mselim00 force-pushed the correct-clocksource branch from c72e3f2 to dfcbe65 Compare February 7, 2025 06:09
@ndbaker1
Copy link
Member

ndbaker1 commented Feb 8, 2025

/ci

Copy link
Contributor

github-actions bot commented Feb 8, 2025

@ndbaker1 roger that! I've dispatched a workflow. 👍

Copy link
Contributor

github-actions bot commented Feb 8, 2025

@ndbaker1 the workflow that you requested has completed. 🎉

AMI variant

@ndbaker1
Copy link
Member

ndbaker1 commented Feb 8, 2025

trying this again :)

/ci

@awslabs awslabs deleted a comment from github-actions bot Feb 8, 2025
@awslabs awslabs deleted a comment from github-actions bot Feb 8, 2025
@ndbaker1
Copy link
Member

ndbaker1 commented Feb 8, 2025

/ci

Copy link
Contributor

github-actions bot commented Feb 8, 2025

@ndbaker1 roger that! I've dispatched a workflow. 👍

Copy link
Contributor

github-actions bot commented Feb 8, 2025

@ndbaker1 the workflow that you requested has completed. 🎉

AMI variantBuildTest
1.25 / al2success ✅success ✅
1.25 / al2023success ✅success ✅
1.26 / al2success ✅success ✅
1.26 / al2023success ✅success ✅
1.27 / al2success ✅success ✅
1.27 / al2023success ✅success ✅
1.28 / al2success ✅success ✅
1.28 / al2023success ✅success ✅
1.29 / al2success ✅success ✅
1.29 / al2023success ✅success ✅
1.30 / al2success ✅success ✅
1.30 / al2023success ✅success ✅
1.31 / al2success ✅failure ❌
1.31 / al2023success ✅success ✅
1.32 / al2success ✅success ✅
1.32 / al2023success ✅success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants