cp: fix possible OOM and partial write with large files#6694
cp: fix possible OOM and partial write with large files#6694BenWiederhake merged 1 commit intouutils:mainfrom neyo8826:copy_fix
Conversation
src/uu/cp/src/platform/linux.rs
Outdated
| } | ||
| let src_fd = src_file.as_raw_fd(); | ||
| let mut current_offset: isize = 0; | ||
| let step = std::cmp::min(size, 16 * 1024 * 1024) as usize; // 16 MiB |
There was a problem hiding this comment.
please add a comment to explain why you are doing this
| current_offset.try_into().unwrap(), | ||
| ) | ||
| }; | ||
| for i in (0..len).step_by(step) { |
There was a problem hiding this comment.
same, please document this a bit more :) (i know it wasn't documented before :)
|
thanks |
|
More docs :) I have "benchmarked" it with my large file (technically not sparse, but heuristic triggered it I suppose). |
|
any idea of the impact on slow harddrive ? |
|
Well, if the copy is between 2 HDDs, then it shouldnt matter. 16MB seems a lot, it should balance out anything with the FS help (just my opinion) |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
do you know how I could reproduce the OOM ? thanks |
create a file and copy it with |
BenWiederhake
left a comment
BenWiederhake
left a comment
There was a problem hiding this comment.
LGTM!
Bonus points if you can come up with a way to test this. For example: Create a 500 MB "hole-only" file, and copy it while a ulimit -m 300000 applies.
Please let me know if you intend to write such a test or not.
|
A hole-only file would not trigger any actual reads or writes because of |
BenWiederhake
left a comment
BenWiederhake
left a comment
There was a problem hiding this comment.
Ideally someone should eventually™ come up with a way to test this (without introducing CI flakes), but this is good enough for now. Thanks! :)
3 participants
There are 2 fixes:
pwritedoes not guarantee that all bytes will be written,write_all_atis a proper wrapper over itsparse_copy_without_hole(triggered on ZFS) can eat RAM if file is large and there are not enough holes; now the write happens at max 16 MiB chunks (as far as I tested, it saturates SSD anyways without much CPU)I have tested it with checking sha256sum of a 4 GiB file.