Skip to content

kernel-patches: introduce kernel patch for fscache ondemand IO path o… #717

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

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

kevinXYin
Copy link
Contributor

This is kernel patch for #714

before we figure out a formal solution for upstream, we should fix this performance issue , that may affect the user experience first.

@anolis-bot
Copy link
Collaborator

@kevinXYin , a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/20656

@anolis-bot
Copy link
Collaborator

@kevinXYin , The CI test is completed, please check result:

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus✅ SUCCESS
compile-ctr-remote✅ SUCCESS
compile-nydus-snapshotter✅ SUCCESS
start-nydus-snapshotter-config-containerd✅ SUCCESS
run-container-with-nydus-image✅ SUCCESS

Congratulations, your test job passed!

@hsiangkao
Copy link
Contributor

hsiangkao commented Sep 8, 2022

Yeah, that is a practical way for downstream users.
The Linux community is generally reluctant to mix buffered and directed I/O, so we have to implement a direct I/O approach for upstream instead.

+ break;
+
+ if (!PageUptodate(page)) {
+ error = -EFAULT;
Copy link
Contributor

Choose a reason for hiding this comment

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

-EIO ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will not cause EIO , if we can not find all data in pagecache , will fallback to dio for the rest iter

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, anyway, the error code is somewhat confusing to me... ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah , this is a bit confusing since we don't return result codes here , how about just set this like err = -1 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

let's leave as it for now...

+ break;
+
+ if (!PageUptodate(page)) {
+ error = -EFAULT;
Copy link
Contributor

Choose a reason for hiding this comment

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

okay, anyway, the error code is somewhat confusing to me... ;)

@hsiangkao hsiangkao merged commit d6b0df6 into dragonflyoss:master Sep 8, 2022
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