Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

mount: Fix hugepages support #872

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

bpradipt
Copy link
Contributor

Fixes: #871

Signed-off-by: Pradipta Banerjee pradipta.banerjee@gmail.com

@bpradipt bpradipt changed the title mount: Fix hugepages support [WIP] mount: Fix hugepages support Dec 22, 2020
Copy link
Member

@egernst egernst left a comment

Choose a reason for hiding this comment

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

This part seems more straight forward... though I see one issue (need to return error). The hard part is making sure "these" huge pages are backed by "actual" huge pages from the host.

mount.go Outdated Show resolved Hide resolved
mount.go Outdated Show resolved Hide resolved
@bpradipt bpradipt force-pushed the 1.x-hugepages branch 2 times, most recently from 7a0a500 to 2ea0047 Compare February 2, 2021 09:04
@bpradipt bpradipt changed the title [WIP] mount: Fix hugepages support mount: Fix hugepages support Feb 2, 2021
@bpradipt bpradipt added the needs-forward-port Changes need to be applied to a newer branch / repository label Feb 2, 2021
@bpradipt bpradipt force-pushed the 1.x-hugepages branch 2 times, most recently from 0cc411b to d968f31 Compare February 3, 2021 17:45
@bpradipt
Copy link
Contributor Author

bpradipt commented Feb 3, 2021

/test-ubuntu

@bpradipt
Copy link
Contributor Author

bpradipt commented Feb 4, 2021

/test-vfio

mount.go Outdated Show resolved Hide resolved
mount.go Outdated Show resolved Hide resolved
mount.go Outdated Show resolved Hide resolved
Mount hugepage directories and configure the requested number of hugepages
dynamically by writing to sysfs files

Fixes: kata-containers#871

Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
@bpradipt
Copy link
Contributor Author

/test-ubuntu

@bpradipt bpradipt requested a review from jodh-intel February 17, 2021 10:03
@bpradipt
Copy link
Contributor Author

/test-vfio

1 similar comment
@bpradipt
Copy link
Contributor Author

/test-vfio

@bpradipt
Copy link
Contributor Author

The vfio test failure is unrelated and looks like this is the cause /usr/bin/cloud-hypervisor does not exist .

@dgibson @jodh-intel can you please take a look at this PR. I have incorporated all the changes. Post your review I can start working on forward porting the hugepages changes to 2.x. Thanks

@egernst
Copy link
Member

egernst commented Feb 23, 2021

I have some questions still, from kata-containers/runtime#2353 (comment)

I’m okay with adding support, but I think it should be behind a feature gate, since I think this may cause issues in Kube (our memory overhead will consume a lot of the pages).

Copy link
Contributor

@dgibson dgibson left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

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

Thanks @bpradipt.

lgtm

@bpradipt
Copy link
Contributor Author

I have some questions still, from kata-containers/runtime#2353 (comment)

I’m okay with adding support, but I think it should be behind a feature gate, since I think this may cause issues in Kube (our memory overhead will consume a lot of the pages).

Sorry @egernst, I completely missed it. I have posted few questions and let's discuss to arrive at a consensus.

@fidencio
Copy link
Member

/test-vfio

@bpradipt
Copy link
Contributor Author

bpradipt commented Apr 8, 2021

/test-vfio

@devimc
Copy link

devimc commented Apr 8, 2021

hold on please, the CI didn't run

@devimc devimc added the do-not-merge PR has problems or depends on another label Apr 8, 2021
@devimc
Copy link

devimc commented Apr 8, 2021

I'm investigating the issue

INFO: No forcing label found
Short circuit fast path skipping the rest of the CI.

@devimc
Copy link

devimc commented Apr 8, 2021

@bpradipt - I see you want to merge this in master

bpradipt wants to merge 1 commit into kata-containers:master

is that correct?

@bpradipt
Copy link
Contributor Author

bpradipt commented Apr 8, 2021

@bpradipt - I see you want to merge this in master

bpradipt wants to merge 1 commit into kata-containers:master

is that correct?

This is for kata-containers/agent master (1.x)

@devimc devimc removed the do-not-merge PR has problems or depends on another label Apr 8, 2021
@devimc
Copy link

devimc commented Apr 8, 2021

CI is buggy, no changes were detected 😞

+ info 'Checking for any changed files that will prevent CI fastpath return'
+ echo -e 'INFO: Checking for any changed files that will prevent CI fastpath return'
INFO: Checking for any changed files that will prevent CI fastpath return
++ can_we_skip
++ '[' '' = true ']'
+++ get_pr_changed_file_details_full
+++ local filters=
+++ filters+=A
+++ filters+=C
+++ filters+=M
+++ filters+=R
+++ filters+=UX
+++ git diff-tree -r --name-status --diff-filter=ACMRUX origin/master HEAD
++ filenames=
+++ echo ''
+++ awk '{print $NF}'
++ filenames=
++ '[' -z '' ']'
++ local_info 'No files found'
++ msg='No files found'
++ info 'No files found'
++ echo -e 'INFO: No files found'
INFO: No files found

@devimc devimc added the force-ci Force the CI to run for a PR even if it would normally be skipped label Apr 8, 2021
@devimc
Copy link

devimc commented Apr 8, 2021

/test-ubuntu
/test-vfio

@fidencio
Copy link
Member

/test-ubuntu

@fidencio
Copy link
Member

On a side note, I decided to take a look at the jenkins-ci-ubuntu-18-04 and it's building the rust agent. Why? Oo

@fidencio fidencio merged commit e1941ee into kata-containers:master Apr 22, 2021
@bpradipt bpradipt deleted the 1.x-hugepages branch February 8, 2022 13:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
force-ci Force the CI to run for a PR even if it would normally be skipped needs-forward-port Changes need to be applied to a newer branch / repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hugepage support in Kata
6 participants