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

cleanup: nuke unused scripts, Makefile bits and defunct functionality. #135

Conversation

klihub
Copy link
Collaborator

@klihub klihub commented Oct 11, 2023

This patch set nukes

  • unused docker-build-image script and a few unused Makefile variables
  • defunct page-migration controller/controls (obsoleted by memtierd)
  • defunct memory controller/controls (related kernel control knobs never upstreamed)
  • defunct introspection API (visualization already removed earlier)
  • defunct metrics-based rebalancing

scripts/build/docker-build-image is unused. Let's remove it.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Remove page-migration controller and controls. Some of the necessary
infra has not been fully enabled over the transition away from a CRI
proxy and the functionality has been anyway obsoleted by memtierd.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Remove defunct memory controller. It is mostly obsolete and the
necessary related kernel controls have never been upstreamed,

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Remove the unused introspection interface. It was originally
used by the policy and workload placement visualization proto
which we have already removed earlier.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Remove rebalancing policy interface and the related resmgr bits
(timer, event). In its current form it was non-functional anyway
with the timer and rebalance-triggering event disabled in resmgr.
Most policies did not implement rebalancing either.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Copy link
Collaborator

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Love it ❤️
LGTM

@kad
Copy link
Collaborator

kad commented Oct 11, 2023

I'm worried about rebalance infra of code. Probably need to make a backlog item to redesign it to work as expected in NRI scope.

@marquiz
Copy link
Collaborator

marquiz commented Oct 11, 2023

I'm worried about rebalance infra of code. Probably need to make a backlog item to redesign it to work as expected in NRI scope.

I think it's best to nuke the unused/untested code. Create a new GitHub issue about enabling it we want it back.

EDIT: we still have the code in git history 😅

Copy link
Collaborator

@fmuyassarov fmuyassarov left a comment

Choose a reason for hiding this comment

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

LGTM

@marquiz marquiz merged commit 66ed9f4 into containers:main Oct 11, 2023
2 checks passed
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.

4 participants