Skip to content

Support fallocate on Linux#5147

Merged
RalfJung merged 1 commit into
rust-lang:masterfrom
LorrensP-2158466:linux-fallocate
Jul 12, 2026
Merged

Support fallocate on Linux#5147
RalfJung merged 1 commit into
rust-lang:masterfrom
LorrensP-2158466:linux-fallocate

Conversation

@LorrensP-2158466

@LorrensP-2158466 LorrensP-2158466 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Closes #5137

This PR implements the linux syscall fallocate and its 64-bit version. It is an extended version of posix_fallocate. So for now this pr only implements the posix_fallocate behaviour as explained in the man page.

All other "features" are rather specific and I couldn't find a reliable way to mimic them in Miri, they also specify that they require file system support, so i have no clue there.

If needed, I can include them here or in a follow-up PR.

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Jun 30, 2026
@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Jun 30, 2026
@rustbot

rustbot commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Jun 30, 2026
@LorrensP-2158466

This comment was marked as off-topic.

@RalfJung RalfJung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks a lot! And sorry for the slow review, I was dealing with a $job deadline.
@rustbot author

View changes since this review

Comment thread tests/pass-dep/libc/libc-fs.rs Outdated
Comment thread src/shims/unix/fs.rs Outdated
Comment thread src/shims/unix/fs.rs Outdated
Comment thread src/shims/unix/fs.rs Outdated
Comment thread src/shims/unix/fs.rs
offset: i64,
size: i64,
) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let this = self.eval_context_mut();
// This is mostly a copy of `posix_fallocate` except that errors are returned via errno.
let this = self.eval_context_mut();

Any idea how we could avoid all this code duplication?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

3rd commit shows my vision of it.

Comment thread tests/pass-dep/libc/libc-fs.rs Outdated
// libc::off_t is i32 in target i686-unknown-linux-gnu
// https://docs.rs/libc/latest/i686-unknown-linux-gnu/libc/type.off_t.html

let test_errors = || {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are you packing these into closures?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

posix_fallocate does the same and that was accepted and never changed (my pr).

I can remove both uses of closures.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

split them up into 2 separate test functions. posix_fallocate is left untouched.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah it is hard to apply consistent standards, especially in tests. ;)

But please just use comments or top-level functions for grouping here.

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Jul 11, 2026
@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Jul 11, 2026

@RalfJung RalfJung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I like it :)
@rustbot author

View changes since this review

Comment thread tests/pass-dep/libc/libc-fs.rs Outdated
Comment thread src/shims/unix/fs.rs Outdated
Comment thread src/shims/unix/fs.rs Outdated
Comment thread src/shims/io_error.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Jul 11, 2026
@rustbot

This comment has been minimized.

@LorrensP-2158466

LorrensP-2158466 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@RalfJung I have a feeling something is broken? When I try to fetch and rebase i get a merge conflict which seems easy to fix. Even though its weird i get a conflict in the first place since i didn't change anything in that file other than the rename of the io_error_to_errnum function.

And then when I fix that conflict i get the following:

~/oss/miri @41ba66fe9 rebase-i 7/7 ~1 +3 >> ./miri test --target i686-unknown-linux-gnu libc-fs.rs

error[E0425]: cannot find value `TooManyOpenFiles` in this scope
   --> src/shims/io_error.rs:105:20
    |
105 |         ("EMFILE", TooManyOpenFiles),
    |                    ^^^^^^^^^^^^^^^^ not found in this scope

error[E0425]: cannot find value `TooManyOpenFiles` in this scope
   --> src/shims/io_error.rs:107:20
    |
107 |         ("ENFILE", TooManyOpenFiles),
    |                    ^^^^^^^^^^^^^^^^ not found in this scope

error[E0425]: cannot find value `TooManyOpenFiles` in this scope
   --> src/shims/io_error.rs:262:39
    |
262 |         ("ERROR_TOO_MANY_OPEN_FILES", TooManyOpenFiles),
    |                                       ^^^^^^^^^^^^^^^^ not found in this scope

error[E0635]: unknown feature `io_error_too_many_open_files`
  --> src/lib.rs:21:37
   |
21 | #![cfg_attr(not(bootstrap), feature(io_error_too_many_open_files))]
   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0769]: tuple variant `rustc_abi::BackendRepr::ScalarPair` written as struct variant
   --> src/shims/native_lib/mod.rs:316:33
    |
316 | ...                   let rustc_abi::BackendRepr::ScalarPair { a: _, b: _, b_offset } =
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
help: use the tuple variant pattern syntax instead
    |
316 -                             let rustc_abi::BackendRepr::ScalarPair { a: _, b: _, b_offset } =
316 +                             let rustc_abi::BackendRepr::ScalarPair(_, _) =
    |

I expect that these would be a conflict as well, but that is not the case?

@RalfJung

Copy link
Copy Markdown
Member

Well, try pushing something that resolves the conflicts and we can see what happens. Did you remember to ./miri toolchain?

@rustbot

rustbot commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

Did you remember to ./miri toolchain

Oops... It has been some time 😅.

rebased to fix conflict and address second round of comments, @rustbot ready.

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Jul 11, 2026
@RalfJung

Copy link
Copy Markdown
Member

This looks great, thanks! Please squash the commits. You can squash manually if there are multiple independent commits you want to preserve, or use ./miri squash (make sure to pick a suitable commit message). Then write @rustbot ready after you force-pushed the squashed PR.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Jul 12, 2026
…is is the same behaviour as `posix_fallocate`.

And add tests for `fallocate` based on `posix_fallocate`.
@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

1 commit is fine, I think. @rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Jul 12, 2026
@RalfJung
RalfJung enabled auto-merge July 12, 2026 11:20
@RalfJung RalfJung changed the title Linux fallocate Support fallocate on Linux Jul 12, 2026
@RalfJung
RalfJung added this pull request to the merge queue Jul 12, 2026
Merged via the queue into rust-lang:master with commit 6347e41 Jul 12, 2026
14 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Waiting for a review to complete label Jul 12, 2026
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.

fallocate64 not supported

3 participants