Use LOOP_CONFIGURE when creating loop devices #9211
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @alex-matei. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ad89f0d to
d25502f
Compare
dcantah
reviewed
Oct 10, 2023
|
Cool! Do you have any benchmarks for this change that are enticing? |
henry118
reviewed
Oct 10, 2023
|
@dcantah I ran a few benchmarks: BeforeKubernetes - blockfile snapshotter - 28 pods started in parallel: Ctr: AfterKubernetes - blockfile snapshotter - 28 pods started in parallel: Ctr: |
c4138f8 to
888ceba
Compare
thaJeztah
reviewed
Oct 11, 2023
mount/losetup_linux.go
Outdated
Comment on lines
49
to
50
| func ioctlConfigure(fd int, value *LoopConfig) error { | ||
| _, _, err := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), uintptr(LOOP_CONFIGURE), uintptr(unsafe.Pointer(value))) |
There was a problem hiding this comment.
Is this something that could eventually be upstreamed to golang.org/x/sys?
There was a problem hiding this comment.
Yeah, It might be useful to add it in there as well. I'll check what is the submission flow for it.
There was a problem hiding this comment.
I know maintainers there have been quite receptive to such changes, but much of the code is generated, so it may indeed require a small amount of looking at existing examples to see how to create the patch. @tklauser (👋 sorry for the random ping ❤️) may be able to provide some guidance if needed
There was a problem hiding this comment.
Thanks, it looks pretty straightforward, I made the changes in code. I need to get the CLA signed by legal department and after I'll open a pull request.
fuweid
approved these changes
Oct 13, 2023
There was a problem hiding this comment.
LGTM!
In my local, it's really good.
goos: linux
goarch: amd64
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
➜ mount git:(review-9211) sudo $(command -v go) test -test.root -run TestAttachDetachLoopDevice -count=1000 -failfast ./...
PASS
ok github.com/containerd/containerd/mount 19.471s
➜ mount git:(main) sudo $(command -v go) test -test.root -run TestAttachDetachLoopDevice -count=1000 -failfast ./...
PASS
ok github.com/containerd/containerd/mount 46.858s
888ceba to
27d30df
Compare
samuelkarp
reviewed
Oct 16, 2023
mount/losetup_linux.go
Outdated
| @@ -32,8 +35,26 @@ const ( | |||
| loopDevFormat = "/dev/loop%d" | |||
|
|
|||
| ebusyString = "device or resource busy" | |||
|
|
|||
| LOOP_CONFIGURE = 0x4c0a //nolint: revive | |||
There was a problem hiding this comment.
This const doesn't need to be exported and the convention would be to use mixed caps.
Suggested change
| LOOP_CONFIGURE = 0x4c0a //nolint: revive | |
| loopConfigureIoctl = 0x4c0a //nolint: revive |
27d30df to
4e2afd2
Compare
LOOP_CONFIGURE is a new ioctl that is a lot faster than the LOOP_SET_FD+LOOP_SET_STATUS64 calls Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>
4e2afd2 to
a782fd6
Compare
samuelkarp
approved these changes
Oct 17, 2023
kiashok
added a commit
to kiashok/containerd
that referenced
this pull request
Oct 23, 2024
…h upstream containerd/main update fork-external/main branch to upstream containerd/main at commit f90f80d Related work items: containerd#8736, containerd#8861, containerd#8924, containerd#8934, containerd#9027, containerd#9076, containerd#9104, containerd#9118, containerd#9141, containerd#9155, containerd#9177, containerd#9183, containerd#9184, containerd#9186, containerd#9187, containerd#9191, containerd#9200, containerd#9205, containerd#9211, containerd#9214, containerd#9215, containerd#9221, containerd#9223, containerd#9228, containerd#9231, containerd#9234, containerd#9242, containerd#9246, containerd#9247, containerd#9251, containerd#9253, containerd#9254, containerd#9255, containerd#9268
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LOOP_CONFIGURE is a new ioctl that is a lot faster than the LOOP_SET_FD+LOOP_SET_STATUS64 calls.