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

Implement linux copy algorithms using syscalls #312

Merged
merged 4 commits into from
Jan 30, 2024
Merged

Implement linux copy algorithms using syscalls #312

merged 4 commits into from
Jan 30, 2024

Conversation

liss-h
Copy link
Contributor

@liss-h liss-h commented Jan 22, 2024

This PR implements the various linux file copy algorithms using syscalls instead of relying on cp.

Implemented algorithms:

  • file clone using ioctl(2)
  • sparse copy
  • in-kernel dense copy using copy_file_range

Additionally, adds a fallback mechanism to the sparse copy and clone function that tries the "worse" options after failing to use the "good" algorithm. E.g. clone tries clone -> sparse_copy -> in-kernel dense copy -> dense-copy. And fails only if all of them fail.

Edit: this is related to #297

@KIwabuchi KIwabuchi self-requested a review January 22, 2024 22:50
@KIwabuchi
Copy link
Member

Thanks for the PRs! I was swamped with other things last week. I'll finish the reviews (including the other one) this week. Bear with me...

Copy link
Member

@KIwabuchi KIwabuchi left a comment

Choose a reason for hiding this comment

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

This PR looks great to me! I like the new sparse copy functions and the fall-back policy.

I left just a few minor comments.
In addition to that, could you apply clang-format on your side?
There is a clang-format file in the root directory.

verification/sparse_copy/CMakeLists.txt Outdated Show resolved Hide resolved
include/metall/detail/file.hpp Outdated Show resolved Hide resolved
include/metall/detail/file.hpp Show resolved Hide resolved
@liss-h
Copy link
Contributor Author

liss-h commented Jan 30, 2024

Thanks for reviewing

Regarding running clang-format: the style in the .clang-format file does not seem to match the codebase completely. For instance in .clang-format reference and pointer alignment is configured to left but in the codebase it is actually right so running it produces a lot of changes that are unrelated to this PR.

@KIwabuchi
Copy link
Member

That's a catch! I investigated the .clang-format file and found that setting DerivePointerAlignment to true ignores the PointerAlignment option. As it's confusing, I'll update the entire code so that it matches with the 'left' style.

@KIwabuchi
Copy link
Member

I'll merge this PR once the CI has passed.

@KIwabuchi KIwabuchi merged commit 481576b into LLNL:develop Jan 30, 2024
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.

2 participants